From 9f0569cd0b0abeccc59f30c94b92ecb1e579f8d6 Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Wed, 5 May 2021 21:53:25 +0300 Subject: [PATCH] refactoring --- application/Espo/Controllers/Email.php | 38 ++- application/Espo/Controllers/Kanban.php | 2 +- application/Espo/Controllers/User.php | 25 +- .../Espo/Core/Controllers/RecordBase.php | 5 +- application/Espo/Core/Record/Collection.php | 4 + .../Espo/Core/Record/SearchParamsFetcher.php | 2 +- .../Modules/Crm/Controllers/Activities.php | 6 +- .../Espo/Modules/Crm/Services/Activities.php | 42 +-- application/Espo/Services/Record.php | 240 ++++++------------ application/Espo/Tools/Kanban/Kanban.php | 31 +-- .../Espo/Tools/Kanban/KanbanService.php | 3 +- 11 files changed, 165 insertions(+), 233 deletions(-) diff --git a/application/Espo/Controllers/Email.php b/application/Espo/Controllers/Email.php index cf4a80f436..f309e33e12 100644 --- a/application/Espo/Controllers/Email.php +++ b/application/Espo/Controllers/Email.php @@ -36,6 +36,7 @@ use Espo\Core\Exceptions\NotFound; use Espo\Core\{ Controllers\Record, Api\Request, + Select\SearchParams, }; use StdClass; @@ -266,26 +267,25 @@ class Email extends Record return $this->getRecordService()->getFoldersNotReadCounts(); } - protected function fetchSearchParamsFromRequest(Request $request): array + protected function fetchSearchParamsFromRequest(Request $request): SearchParams { - $params = parent::fetchSearchParamsFromRequest($request); + $searchParams = parent::fetchSearchParamsFromRequest($request); $folderId = $request->getQueryParam('folderId'); - if ($folderId) { - $params['where'] = $params['where'] ?? []; - - $params['where'][] = [ - 'type' => 'inFolder', - 'attribute' => 'folderId', - 'value' => $folderId, - ]; - - // @todo Remove the line. - $params['folderId'] = $request->getQueryParam('folderId'); + if (!$folderId) { + return $searchParams; } - return $params; + $where = $searchParams->getWhere() ?? []; + + $where[] = [ + 'type' => 'inFolder', + 'attribute' => 'folderId', + 'value' => $folderId, + ]; + + return $searchParams->withWhere($where); } public function postActionMoveToFolder(Request $request) @@ -295,13 +295,11 @@ class Email extends Record if (!empty($data->ids)) { $idList = $data->ids; } + else if (!empty($data->id)) { + $idList = [$data->id]; + } else { - if (!empty($data->id)) { - $idList = [$data->id]; - } - else { - throw new BadRequest(); - } + throw new BadRequest(); } if (empty($data->folderId)) { diff --git a/application/Espo/Controllers/Kanban.php b/application/Espo/Controllers/Kanban.php index 4c525e99b3..bd32135d1d 100644 --- a/application/Espo/Controllers/Kanban.php +++ b/application/Espo/Controllers/Kanban.php @@ -55,7 +55,7 @@ class Kanban { $entityType = $request->getRouteParam('entityType'); - $searchParams = $this->searchParamsFetcher->fetchRaw($request); + $searchParams = $this->searchParamsFetcher->fetch($request); $result = $this->service->getData($entityType, $searchParams); diff --git a/application/Espo/Controllers/User.php b/application/Espo/Controllers/User.php index 22fd5927bc..ddc8ca87a5 100644 --- a/application/Espo/Controllers/User.php +++ b/application/Espo/Controllers/User.php @@ -37,6 +37,7 @@ use Espo\Core\Exceptions\BadRequest; use Espo\Core\{ Controllers\Record, Api\Request, + Select\SearchParams, }; use StdClass; @@ -167,22 +168,24 @@ class User extends Record } } - protected function fetchSearchParamsFromRequest(Request $request): array + protected function fetchSearchParamsFromRequest(Request $request): SearchParams { - $params = parent::fetchSearchParamsFromRequest($request); + $searchParams = parent::fetchSearchParamsFromRequest($request); $userType = $request->getQueryParam('userType'); - if ($userType) { - $params['where'] = $params['where'] ?? []; - - $params['where'][] = [ - 'type' => 'isOfType', - 'attribute' => 'id', - 'value' => $userType, - ]; + if (!$userType) { + return $searchParams; } - return $params; + $where = $searchParams->getWhere() ?? []; + + $where[] = [ + 'type' => 'isOfType', + 'attribute' => 'id', + 'value' => $userType, + ]; + + return $searchParams->withWhere($where); } } diff --git a/application/Espo/Core/Controllers/RecordBase.php b/application/Espo/Core/Controllers/RecordBase.php index 9721a92fb5..18cc6d3444 100644 --- a/application/Espo/Core/Controllers/RecordBase.php +++ b/application/Espo/Core/Controllers/RecordBase.php @@ -48,6 +48,7 @@ use Espo\Core\{ Api\Request, Api\Response, Record\Crud as CrudService, + Select\SearchParams, Di, }; @@ -243,9 +244,9 @@ class RecordBase extends Base implements Di\EntityManagerAware return true; } - protected function fetchSearchParamsFromRequest(Request $request): array + protected function fetchSearchParamsFromRequest(Request $request): SearchParams { - return $this->searchParamsFetcher->fetchRaw($request); + return $this->searchParamsFetcher->fetch($request); } public function postActionExport(Request $request): StdClass diff --git a/application/Espo/Core/Record/Collection.php b/application/Espo/Core/Record/Collection.php index 3dfbcf97d5..be1aaacc5c 100644 --- a/application/Espo/Core/Record/Collection.php +++ b/application/Espo/Core/Record/Collection.php @@ -40,6 +40,10 @@ use StdClass; */ class Collection { + public const TOTAL_HAS_MORE = -1; + + public const TOTAL_HAS_NO_MORE = -2; + private $collection; private $total; diff --git a/application/Espo/Core/Record/SearchParamsFetcher.php b/application/Espo/Core/Record/SearchParamsFetcher.php index 13a4572a2c..0b49b1f3d6 100644 --- a/application/Espo/Core/Record/SearchParamsFetcher.php +++ b/application/Espo/Core/Record/SearchParamsFetcher.php @@ -54,7 +54,7 @@ class SearchParamsFetcher ); } - public function fetchRaw(Request $request): array + private function fetchRaw(Request $request): array { $params = []; diff --git a/application/Espo/Modules/Crm/Controllers/Activities.php b/application/Espo/Modules/Crm/Controllers/Activities.php index 1938dfc067..5169c1b7a7 100644 --- a/application/Espo/Modules/Crm/Controllers/Activities.php +++ b/application/Espo/Modules/Crm/Controllers/Activities.php @@ -45,6 +45,8 @@ use Espo\Modules\Crm\Services\Activities as Service; use Espo\Entities\User; +use StdClass; + class Activities { private const MAX_CALENDAR_RANGE = 123; @@ -260,7 +262,7 @@ class Activities ]); } - public function getActionEntityTypeList(Request $request) + public function getActionEntityTypeList(Request $request): StdClass { $params = $request->getRouteParams(); @@ -295,7 +297,7 @@ class Activities throw new BadRequest(); } - $searchParams = $this->searchParamsFetcher->fetchRaw($request); + $searchParams = $this->searchParamsFetcher->fetch($request); $result = $this->service->findActivitiyEntityType($scope, $id, $entityType, $isHistory, $searchParams); diff --git a/application/Espo/Modules/Crm/Services/Activities.php b/application/Espo/Modules/Crm/Services/Activities.php index e4edee4f0d..18bc801c4d 100644 --- a/application/Espo/Modules/Crm/Services/Activities.php +++ b/application/Espo/Modules/Crm/Services/Activities.php @@ -46,6 +46,7 @@ use Espo\Core\{ FieldProcessing\ListLoadProcessor, FieldProcessing\LoaderParams as FieldLoaderParams, Di, + Record\ServiceContainer as RecordServiceContainer, }; use Espo\{ @@ -87,12 +88,16 @@ class Activities implements private $listLoadProcessor; + private $recordServiceContainer; + public function __construct( WhereConverterFactory $whereConverterFactory, - ListLoadProcessor $listLoadProcessor + ListLoadProcessor $listLoadProcessor, + RecordServiceContainer $recordServiceContainer ) { $this->whereConverterFactory = $whereConverterFactory; $this->listLoadProcessor = $listLoadProcessor; + $this->recordServiceContainer = $recordServiceContainer; } protected function getPDO() @@ -142,8 +147,10 @@ class Activities implements } protected function getActivitiesUserMeetingQuery( - Entity $entity, array $statusList = []/*, $isHistory = false*//*, $additinalSelectParams = null*/ + Entity $entity, + array $statusList = [] ) { + $builder = $this->selectBuilderFactory ->create() ->from('Meeting') @@ -754,8 +761,8 @@ class Activities implements string $scope, string $id, string $entityType, - bool $isHistory = false, - array $params = [] + bool $isHistory, + SearchParams $searchParams ): RecordCollection { if (!$this->getAcl()->checkScope($entityType)) { @@ -781,18 +788,16 @@ class Activities implements $statusList = $this->getMetadata()->get(['scopes', $entityType, 'historyStatusList'], ['Held', 'Not Held']); } - $service = $this->getServiceFactory()->create($entityType); - - if ($entityType === 'Email') { - if ($params['orderBy'] ?? null === 'dateStart') { - $params['orderBy'] = 'dateSent'; - } + if ($entityType === 'Email' && $searchParams->getOrderBy() === 'dateStart') { + $searchParams = $searchParams->withOrderBy('dateSent'); } - $service->handleListParams($params); + $service = $this->recordServiceContainer->get($entityType); - $offset = $params['offset'] ?? null; - $limit = $params['maxSize'] ?? null; + $preparedSearchParams = $service->prepareSearchParams($searchParams); + + $offset = $searchParams->getOffset(); + $limit = $searchParams->getMaxSize(); $baseQueryList = $this->getActivitiesQuery($entity, $entityType, $statusList); @@ -804,13 +809,11 @@ class Activities implements $order = null; - $searchParams = SearchParams::fromRaw($params); - foreach ($baseQueryList as $i => $itemQuery) { $itemBuilder = $this->selectBuilderFactory ->create() ->clone($itemQuery) - ->withSearchParams($searchParams) + ->withSearchParams($preparedSearchParams) ->withComplexExpressionsForbidden() ->withWherePermissionCheck() ->buildQueryBuilder(); @@ -1282,8 +1285,13 @@ class Activities implements } protected function getCalendarQuery( - string $scope, string $userId, string $from, string $to, bool $skipAcl = false + string $scope, + string $userId, + string $from, + string $to, + bool $skipAcl = false ): Select { + if ($this->serviceFactory->checkExists($scope)) { $service = $this->serviceFactory->create($scope); diff --git a/application/Espo/Services/Record.php b/application/Espo/Services/Record.php index 77c51550b3..830235915f 100644 --- a/application/Espo/Services/Record.php +++ b/application/Espo/Services/Record.php @@ -390,15 +390,6 @@ class Record implements Crud, $this->entityManager->saveEntity($historyRecord); } - /** - * @deprecated Use `read` method. - * @todo Remove in 6.3. - */ - public function readEntity($id) - { - return $this->read($id); - } - /** * Read a record by ID. Access control check is performed. * @@ -1042,14 +1033,6 @@ class Record implements Crud, return $entity; } - /** - * @deprecated - */ - public function updateEntity($id, $data) - { - return $this->update($id, $data); - } - /** * Update a record. * @@ -1130,14 +1113,6 @@ class Record implements Crud, { } - /** - * @deprecated - */ - public function deleteEntity($id) - { - return $this->delete($id); - } - public function delete(string $id): void { if (!$this->acl->check($this->entityType, AclTable::ACTION_DELETE)) { @@ -1183,66 +1158,46 @@ class Record implements Crud, return $selectParams; } - /** - * @deprecated - */ - public function findEntities($params) - { - return $this->find($params); - } - /** * Find records. * * @params $params Raw search parameters. * @return RecordCollection */ - public function find(array $params): RecordCollection + public function find(SearchParams $searchParams): RecordCollection { if (!$this->acl->check($this->entityType, AclTable::ACTION_READ)) { throw new ForbiddenSilent(); } - $disableCount = false; + $disableCount = + $this->listCountQueryDisabled || + $this->metadata->get(['entityDefs', $this->entityType, 'collection', 'countDisabled']); - if ( - $this->listCountQueryDisabled - || - $this->metadata->get(['entityDefs', $this->entityType, 'collection', 'countDisabled']) - ) { - $disableCount = true; + $maxSize = $searchParams->getMaxSize(); + + if ($disableCount && $maxSize) { + $searchParams = $searchParams->withMaxSize($maxSize + 1); } - $maxSize = 0; - - if ($disableCount) { - if (!empty($params['maxSize'])) { - $maxSize = $params['maxSize']; - - $params['maxSize'] = $params['maxSize'] + 1; - } - } - - $this->handleListParams($params); - - $searchParams = SearchParams::fromRaw($params); + $preparedSearchParams = $this->prepareSearchParams($searchParams); $selectBuilder = $this->selectBuilderFactory->create(); $query = $selectBuilder ->from($this->entityType) ->withStrictAccessControl() - ->withSearchParams($searchParams) + ->withSearchParams($preparedSearchParams) ->build(); $collection = $this->getRepository() ->clone($query) ->find(); - foreach ($collection as $e) { - $this->loadListAdditionalFields($e, $searchParams); + foreach ($collection as $entity) { + $this->loadListAdditionalFields($entity, $preparedSearchParams); - $this->prepareEntityForOutput($e); + $this->prepareEntityForOutput($entity); } if (!$disableCount) { @@ -1251,12 +1206,12 @@ class Record implements Crud, ->count(); } else if ($maxSize && count($collection) > $maxSize) { - $total = -1; + $total = RecordCollection::TOTAL_HAS_MORE; unset($collection[count($collection) - 1]); } else { - $total = -2; + $total = RecordCollection::TOTAL_HAS_NO_MORE; } return new RecordCollection($collection, $total); @@ -1315,12 +1270,6 @@ class Record implements Crud, self::MAX_SELECT_TEXT_ATTRIBUTE_LENGTH; } - /** @deprecated */ - public function findLinkedEntities($id, $link, $params) - { - return $this->findLinked($id, $link, $params); - } - /** * Find linked records. * @@ -1328,7 +1277,7 @@ class Record implements Crud, * @throws Forbidden If no access. * @throws Error */ - public function findLinked(string $id, string $link, array $params): RecordCollection + public function findLinked(string $id, string $link, SearchParams $searchParams): RecordCollection { if (!$this->acl->check($this->entityType, AclTable::ACTION_READ)) { throw new ForbiddenSilent("No access."); @@ -1344,7 +1293,7 @@ class Record implements Crud, throw new ForbiddenSilent(); } - if (empty($link)) { + if (!$link) { throw new Error("Empty link."); } @@ -1352,14 +1301,8 @@ class Record implements Crud, $methodName = 'findLinked' . ucfirst($link); - if ($link !== 'entities' && method_exists($this, $methodName)) { - return $this->$methodName($id, $params); - } - - $methodName = 'findLinkedEntities' . ucfirst($link); - if (method_exists($this, $methodName)) { - return $this->$methodName($id, $params); + return $this->$methodName($id, $searchParams); } $foreignEntityType = $this->entityManager @@ -1380,45 +1323,28 @@ class Record implements Crud, $recordService = $this->recordServiceContainer->get($foreignEntityType); - $disableCount = false; $disableCountPropertyName = 'findLinked' . ucfirst($link) . 'CountQueryDisabled'; - if ( - !empty($this->$disableCountPropertyName) - ) { - $disableCount = true; + $disableCount = + property_exists($this, $disableCountPropertyName) && + $this->$disableCountPropertyName; + + $maxSize = $searchParams->getMaxSize(); + + if ($disableCount && $maxSize) { + $searchParams = $searchParams->withMaxSize($maxSize + 1); } - $maxSize = 0; - - if ($disableCount) { - if (!empty($params['maxSize'])) { - $maxSize = $params['maxSize']; - $params['maxSize'] = $params['maxSize'] + 1; - } - } - - $recordService->handleListParams($params); - - if (isset($params['select'])) { - $mandatorySelectAttributeList = $this->linkMandatorySelectAttributeList[$link] ?? []; - - foreach ($mandatorySelectAttributeList as $item) { - if (in_array($item, $params['select'])) { - continue; - } - - $params['select'][] = $item; - } - } - - $searchParams = SearchParams::fromRaw($params); + $preparedSearchParams = $this->prepareLinkSearchParams( + $recordService->prepareSearchParams($searchParams), + $link + ); $selectBuilder = $this->selectBuilderFactory->create(); $selectBuilder ->from($foreignEntityType) - ->withSearchParams($searchParams); + ->withSearchParams($preparedSearchParams); if (!$skipAcl) { $selectBuilder->withStrictAccessControl(); @@ -1436,10 +1362,10 @@ class Record implements Crud, ->clone($query) ->find(); - foreach ($collection as $e) { - $this->loadListAdditionalFields($e, $searchParams); + foreach ($collection as $itemEntity) { + $this->loadListAdditionalFields($itemEntity, $preparedSearchParams); - $recordService->prepareEntityForOutput($e); + $recordService->prepareEntityForOutput($itemEntity); } if (!$disableCount) { @@ -1450,25 +1376,17 @@ class Record implements Crud, ->count(); } else if ($maxSize && count($collection) > $maxSize) { - $total = -1; + $total = RecordCollection::TOTAL_HAS_MORE; unset($collection[count($collection) - 1]); } else { - $total = -2; + $total = RecordCollection::TOTAL_HAS_NO_MORE; } return new RecordCollection($collection, $total); } - /** - * @deprecated - */ - public function linkEntity($id, $link, $foreignId) - { - return $this->link($id, $link, $foreignId); - } - /** * Link records. * @@ -1537,14 +1455,6 @@ class Record implements Crud, $this->getRepository()->relate($entity, $link, $foreignEntity); } - /** - * @deprecated - */ - public function unlinkEntity($id, $link, $foreignId) - { - return $this->unlink($id, $link, $foreignId); - } - /** * Unlink records. * @@ -1613,14 +1523,6 @@ class Record implements Crud, $this->getRepository()->unrelate($entity, $link, $foreignEntity); } - /** - * @deprecated - */ - public function linkEntityMass($id, $link, $where, $selectData = null) - { - return $this->massLink($id, $link, $where, $selectData); - } - public function linkFollowers(string $id, string $foreignId): void { if (!$this->acl->check($this->entityType, AclTable::ACTION_EDIT)) { @@ -2248,47 +2150,59 @@ class Record implements Crud, return $this->fieldUtil->getFieldByTypeList($this->entityType, $type); } - /** - * Handle list parameters (passed from front-end). - */ - public function handleListParams(array &$params) + public function prepareSearchParams(SearchParams $searchParams): SearchParams { - $this->handleListParamsSelect($params); - - $params['maxTextAttributeLength'] = $this->getMaxSelectTextAttributeLength(); + return $this + ->prepareSearchParamsSelect($searchParams) + ->withMaxTextAttributeLength( + $this->getMaxSelectTextAttributeLength() + ); } - protected function handleListParamsSelect(array &$params) + protected function prepareSearchParamsSelect(SearchParams $searchParams): SearchParams { if ($this->forceSelectAllAttributes) { - unset($params['select']); - - return; + return $searchParams->withSelect(null); } if ($this->selectAttributeList) { - $params['select'] = $this->selectAttributeList; - - return; + return $searchParams->withSelect($this->selectAttributeList); } - if ( - count($this->mandatorySelectAttributeList) && - array_key_exists('select', $params) && is_array($params['select']) - ) { + if (count($this->mandatorySelectAttributeList) && $searchParams->getSelect() !== null) { + $select = array_unique( + array_merge( + $searchParams->getSelect(), + $this->mandatorySelectAttributeList + ) + ); - $itemList = $params['select']; - - foreach ($this->mandatorySelectAttributeList as $attribute) { - if (in_array($attribute, $itemList)) { - continue; - } - - $itemList[] = $attribute; - } - - $params['select'] = $itemList; + return $searchParams->withSelect($select); } + + return $searchParams; + } + + protected function prepareLinkSearchParams(SearchParams $searchParams, string $link): SearchParams + { + if ($searchParams->getSelect() === null) { + return $searchParams; + } + + $mandatorySelectAttributeList = $this->linkMandatorySelectAttributeList[$link] ?? null; + + if ($mandatorySelectAttributeList === null) { + return $searchParams; + } + + $select = array_unique( + array_merge( + $searchParams->getSelect(), + $mandatorySelectAttributeList + ) + ); + + return $searchParams->withSelect($select); } /** diff --git a/application/Espo/Tools/Kanban/Kanban.php b/application/Espo/Tools/Kanban/Kanban.php index 5767b9d087..a4771a4f82 100644 --- a/application/Espo/Tools/Kanban/Kanban.php +++ b/application/Espo/Tools/Kanban/Kanban.php @@ -55,7 +55,10 @@ class Kanban protected $orderDisabled = false; - protected $searchParams = []; + /** + * @var SearchParams + */ + protected $searchParams = null; protected $userId = null; @@ -92,7 +95,7 @@ class Kanban return $this; } - public function setSearchParams(array $searchParams): self + public function setSearchParams(SearchParams $searchParams): self { $this->searchParams = $searchParams; @@ -138,25 +141,23 @@ class Kanban */ public function getResult(): Result { - $params = $this->searchParams; - if (!$this->entityType) { throw new Error("Entity type is not specified."); } - $recordService = $this->recordServiceContainer->get($this->entityType); - - $maxSize = 0; - - if ($this->countDisabled) { - if (!empty($params['maxSize'])) { - $maxSize = $params['maxSize']; - - $params['maxSize'] = $params['maxSize'] + 1; - } + if (!$this->searchParams) { + throw new Error("No search params."); } - $searchParams = SearchParams::fromRaw($params); + $searchParams = $this->searchParams; + + $recordService = $this->recordServiceContainer->get($this->entityType); + + $maxSize = $searchParams->getMaxSize(); + + if ($this->countDisabled && $maxSize) { + $searchParams = $searchParams->withMaxSize($maxSize + 1); + } $query = $this->selectBuilderFactory ->create() diff --git a/application/Espo/Tools/Kanban/KanbanService.php b/application/Espo/Tools/Kanban/KanbanService.php index f04c27d706..84202eaf1d 100644 --- a/application/Espo/Tools/Kanban/KanbanService.php +++ b/application/Espo/Tools/Kanban/KanbanService.php @@ -36,6 +36,7 @@ use Espo\Core\{ Utils\Metadata, Exceptions\ForbiddenSilent, Acl\Table, + Select\SearchParams, }; use Espo\Entities\User; @@ -70,7 +71,7 @@ class KanbanService $this->orderer = $orderer; } - public function getData(string $entityType, array $searchParams): Result + public function getData(string $entityType, SearchParams $searchParams): Result { $this->processAccessCheck($entityType);