From acd3d06a3337fd62d8d8b846dfa47cd67f3e5df0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D?= Date: Thu, 9 May 2024 09:31:44 +0200 Subject: [PATCH] refactor(php): Decouple view data from domain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rubén D --- .../Helpers/Account}/AccountSearchData.php | 63 +++++++++++-------- .../Helpers/Account/AccountSearchHelper.php | 36 ++++++----- lib/SP/Core/Definitions/DomainDefinitions.php | 4 +- .../Ports/AccountSearchDataBuilder.php | 46 -------------- .../Account/Ports/AccountSearchRepository.php | 6 +- .../Domain/Account/Services/AccountSearch.php | 28 +++------ .../Account/Repositories/AccountSearch.php | 4 +- .../Infrastructure/Database/QueryResult.php | 40 +++++++----- .../Account/Services/AccountSearchTest.php | 63 ++++++------------- .../Account}/AccountSearchDataTest.php | 16 ++--- 10 files changed, 127 insertions(+), 179 deletions(-) rename {lib/SP/Domain/Account/Services/Builders => app/modules/web/Controllers/Helpers/Account}/AccountSearchData.php (81%) delete mode 100644 lib/SP/Domain/Account/Ports/AccountSearchDataBuilder.php rename tests/SP/Domain/{Account/Services/Builders => SP/Tests/Modules/Web/Controllers/Helpers/Account}/AccountSearchDataTest.php (97%) diff --git a/lib/SP/Domain/Account/Services/Builders/AccountSearchData.php b/app/modules/web/Controllers/Helpers/Account/AccountSearchData.php similarity index 81% rename from lib/SP/Domain/Account/Services/Builders/AccountSearchData.php rename to app/modules/web/Controllers/Helpers/Account/AccountSearchData.php index 293530e8..12087214 100644 --- a/lib/SP/Domain/Account/Services/Builders/AccountSearchData.php +++ b/app/modules/web/Controllers/Helpers/Account/AccountSearchData.php @@ -1,4 +1,26 @@ . + */ declare(strict_types=1); /** @@ -24,24 +46,21 @@ declare(strict_types=1); * along with sysPass. If not, see . */ -namespace SP\Domain\Account\Services\Builders; +namespace SP\Modules\Web\Controllers\Helpers\Account; -use SP\Core\Application; use SP\Domain\Account\Adapters\AccountSearchItem; use SP\Domain\Account\Dtos\AccountAclDto; use SP\Domain\Account\Models\AccountSearchView; use SP\Domain\Account\Ports\AccountAclService; use SP\Domain\Account\Ports\AccountCacheService; -use SP\Domain\Account\Ports\AccountSearchDataBuilder; use SP\Domain\Account\Ports\AccountToFavoriteService; use SP\Domain\Account\Ports\AccountToTagRepository; -use SP\Domain\Common\Services\Service; use SP\Domain\Config\Ports\ConfigDataInterface; use SP\Domain\Core\Acl\AclActionsInterface; use SP\Domain\Core\Bootstrap\UriContextInterface; +use SP\Domain\Core\Context\Context; use SP\Domain\Core\Exceptions\ConstraintException; use SP\Domain\Core\Exceptions\QueryException; -use SP\Domain\Core\Exceptions\SPException; use SP\Domain\Storage\Ports\FileCacheService; use SP\Infrastructure\Database\QueryResult; use SP\Infrastructure\File\FileException; @@ -50,9 +69,9 @@ use function SP\logger; use function SP\processException; /** - * Class AccountSearchDataBuilder + * Class AccountSearchData */ -final class AccountSearchData extends Service implements AccountSearchDataBuilder +final class AccountSearchData { private const COLORS_CACHE_FILE = CACHE_PATH . DIRECTORY_SEPARATOR . 'colors.cache'; private const COLORS = [ @@ -79,7 +98,7 @@ final class AccountSearchData extends Service implements AccountSearchDataBuilde private ?array $accountColor = null; public function __construct( - Application $application, + private readonly Context $context, private readonly AccountAclService $accountAclService, private readonly AccountToTagRepository $accountToTagRepository, private readonly AccountToFavoriteService $accountToFavoriteService, @@ -88,8 +107,6 @@ final class AccountSearchData extends Service implements AccountSearchDataBuilde private readonly ConfigDataInterface $configData, private readonly UriContextInterface $uriContext, ) { - parent::__construct($application); - $this->loadColors(); } @@ -108,14 +125,13 @@ final class AccountSearchData extends Service implements AccountSearchDataBuilde } /** - * @param QueryResult $queryResult + * @param QueryResult $queryResult * - * @return AccountSearchItem[] + * @return QueryResult * @throws ConstraintException * @throws QueryException - * @throws SPException */ - public function buildFrom(QueryResult $queryResult): array + public function buildFrom(QueryResult $queryResult): QueryResult { $maxTextLength = $this->configData->isResultsAsCards() ? self::TEXT_LENGTH_CARDS : self::TEXT_LENGTH_NORMAL; $userPreferencesData = $this->context->getUserData()->getPreferences(); @@ -124,16 +140,12 @@ final class AccountSearchData extends Service implements AccountSearchDataBuilde || $this->configData->isAccountLink(); $favorites = $this->accountToFavoriteService->getForUserId($this->context->getUserData()->getId()); - return array_map( - /** - * @param AccountSearchView $accountSearchView - * - * @return AccountSearchItem - * @throws ConstraintException - * @throws QueryException - * @throws SPException - */ - function (AccountSearchView $accountSearchView) use ($maxTextLength, $accountLinkEnabled, $favorites) { + return $queryResult->mutateWithCallback( + function (AccountSearchView $accountSearchView) use ( + $maxTextLength, + $accountLinkEnabled, + $favorites + ): AccountSearchItem { $cache = $this->accountCacheService->getCacheForAccount( $accountSearchView->getId(), strtotime($accountSearchView->getDateEdit()) @@ -169,8 +181,7 @@ final class AccountSearchData extends Service implements AccountSearchDataBuilde $this->pickAccountColor($accountSearchView->getClientId()), $accountLinkEnabled ); - }, - $queryResult->getDataAsArray(AccountSearchView::class) + } ); } diff --git a/app/modules/web/Controllers/Helpers/Account/AccountSearchHelper.php b/app/modules/web/Controllers/Helpers/Account/AccountSearchHelper.php index e9b77cfd..aebd3db6 100644 --- a/app/modules/web/Controllers/Helpers/Account/AccountSearchHelper.php +++ b/app/modules/web/Controllers/Helpers/Account/AccountSearchHelper.php @@ -53,6 +53,8 @@ use SP\Modules\Web\Controllers\Helpers\HelperBase; use SP\Mvc\View\Components\SelectItemAdapter; use SP\Mvc\View\TemplateInterface; +use function SP\getElapsedTime; + /** * Class AccountSearch * @@ -63,26 +65,27 @@ final class AccountSearchHelper extends HelperBase /** * @var bool Indica si el filtrado de cuentas está activo */ - private bool $filterOn = false; - private bool $isAjax = false; - private int $queryTimeStart; + private bool $filterOn = false; + private bool $isAjax = false; + private int $queryTimeStart; private bool $isIndex; private ?AccountSearchFilterDto $accountSearchFilter = null; - private ClientService $clientService; + private ClientService $clientService; private AccountSearchService $accountSearchService; private AccountActionsHelper $accountActionsHelper; - private CategoryService $categoryService; - private TagService $tagService; + private CategoryService $categoryService; + private TagService $tagService; public function __construct( - Application $application, - TemplateInterface $template, - RequestService $request, - ClientService $clientService, - CategoryService $categoryService, - TagService $tagService, - AccountSearchService $accountSearchService, - AccountActionsHelper $accountActionsHelper + Application $application, + TemplateInterface $template, + RequestService $request, + ClientService $clientService, + CategoryService $categoryService, + TagService $tagService, + AccountSearchService $accountSearchService, + AccountActionsHelper $accountActionsHelper, + private readonly AccountSearchData $accountSearchData ) { parent::__construct($application, $template, $request); @@ -243,8 +246,11 @@ final class AccountSearchHelper extends HelperBase ); } + $accountSearchData = $this->accountSearchData->buildFrom( + $this->accountSearchService->getByFilter($this->accountSearchFilter) + ); $dataGrid = $this->getGrid(); - $dataGrid->getData()->setData($this->accountSearchService->getByFilter($this->accountSearchFilter)); + $dataGrid->getData()->setData($accountSearchData); $dataGrid->updatePager(); $dataGrid->setTime(round(getElapsedTime($this->queryTimeStart), 5)); diff --git a/lib/SP/Core/Definitions/DomainDefinitions.php b/lib/SP/Core/Definitions/DomainDefinitions.php index 94e42f57..92f8ea0b 100644 --- a/lib/SP/Core/Definitions/DomainDefinitions.php +++ b/lib/SP/Core/Definitions/DomainDefinitions.php @@ -1,4 +1,5 @@ autowire(AccountSearchData::class), SecureSessionService::class => factory( static function (ContainerInterface $c) { $fileCache = new FileCache( diff --git a/lib/SP/Domain/Account/Ports/AccountSearchDataBuilder.php b/lib/SP/Domain/Account/Ports/AccountSearchDataBuilder.php deleted file mode 100644 index b9f55303..00000000 --- a/lib/SP/Domain/Account/Ports/AccountSearchDataBuilder.php +++ /dev/null @@ -1,46 +0,0 @@ -. - */ - -namespace SP\Domain\Account\Ports; - -use SP\Domain\Account\Adapters\AccountSearchItem; -use SP\Domain\Core\Exceptions\ConstraintException; -use SP\Domain\Core\Exceptions\QueryException; -use SP\Infrastructure\Database\QueryResult; - -/** - * Class AccountSearchDataBuilder - */ -interface AccountSearchDataBuilder -{ - /** - * @param QueryResult $queryResult - * - * @return AccountSearchItem[] - * @throws ConstraintException - * @throws QueryException - */ - public function buildFrom(QueryResult $queryResult): array; -} diff --git a/lib/SP/Domain/Account/Ports/AccountSearchRepository.php b/lib/SP/Domain/Account/Ports/AccountSearchRepository.php index 9dad6156..75171053 100644 --- a/lib/SP/Domain/Account/Ports/AccountSearchRepository.php +++ b/lib/SP/Domain/Account/Ports/AccountSearchRepository.php @@ -1,4 +1,5 @@ */ public function getByFilter(AccountSearchFilterDto $accountSearchFilter): QueryResult; diff --git a/lib/SP/Domain/Account/Services/AccountSearch.php b/lib/SP/Domain/Account/Services/AccountSearch.php index 02612011..0389ec9c 100644 --- a/lib/SP/Domain/Account/Services/AccountSearch.php +++ b/lib/SP/Domain/Account/Services/AccountSearch.php @@ -1,4 +1,5 @@ */ public function getByFilter(AccountSearchFilterDto $accountSearchFilter): QueryResult { @@ -80,12 +77,7 @@ final class AccountSearch extends Service implements AccountSearchService } } - $queryResult = $this->accountSearchRepository->getByFilter($accountSearchFilter); - - return QueryResult::withTotalNumRows( - $this->accountSearchDataBuilder->buildFrom($queryResult), - $queryResult->getTotalNumRows() - ); + return $this->accountSearchRepository->getByFilter($accountSearchFilter); } private function processFilterItems(array $filters): void diff --git a/lib/SP/Infrastructure/Account/Repositories/AccountSearch.php b/lib/SP/Infrastructure/Account/Repositories/AccountSearch.php index 52bcf619..b5ec3f6b 100644 --- a/lib/SP/Infrastructure/Account/Repositories/AccountSearch.php +++ b/lib/SP/Infrastructure/Account/Repositories/AccountSearch.php @@ -1,4 +1,5 @@ * @throws ConstraintException * @throws QueryException */ diff --git a/lib/SP/Infrastructure/Database/QueryResult.php b/lib/SP/Infrastructure/Database/QueryResult.php index 0a4727e7..a06dcc56 100644 --- a/lib/SP/Infrastructure/Database/QueryResult.php +++ b/lib/SP/Infrastructure/Database/QueryResult.php @@ -1,4 +1,5 @@ data = SplFixedArray::fromArray($data); - $this->numRows = $this->data->count(); + if ($data instanceof SplFixedArray) { + $this->data = $data; } else { - $this->data = new SplFixedArray(); - $this->numRows = 0; + $this->data = SplFixedArray::fromArray($data ?? []); } + + $this->numRows = $this->data->count(); } - public static function withTotalNumRows( - array $data, - ?int $totalNumRows = null - ): QueryResult { + public static function withTotalNumRows(array $data, ?int $totalNumRows = null): QueryResult + { $result = new self($data); $result->totalNumRows = (int)$totalNumRows; @@ -92,9 +91,7 @@ class QueryResult && (!is_object($this->data->offsetGet(0)) || !is_a($this->data->offsetGet(0), $dataType)) ) { - throw new TypeError( - sprintf(__u('Invalid data\'s type. Expected: %s'), $dataType) - ); + throw new TypeError(sprintf(__u('Invalid data\'s type. Expected: %s'), $dataType)); } } @@ -109,7 +106,7 @@ class QueryResult $this->checkDataType($dataType); } - return $this->data->toArray(); + return $this->data?->toArray(); } public function getNumRows(): int @@ -131,4 +128,15 @@ class QueryResult { return $this->lastId; } + + /** + * Mutate the current data into another {@link QueryResult} by applying the given callback function + * + * @param callable $callable + * @return QueryResult + */ + public function mutateWithCallback(callable $callable): QueryResult + { + return new self(SplFixedArray::fromArray(array_map($callable, $this->data->toArray()))); + } } diff --git a/tests/SP/Domain/Account/Services/AccountSearchTest.php b/tests/SP/Domain/Account/Services/AccountSearchTest.php index 3a3e1041..24dc5f39 100644 --- a/tests/SP/Domain/Account/Services/AccountSearchTest.php +++ b/tests/SP/Domain/Account/Services/AccountSearchTest.php @@ -1,4 +1,5 @@ with($accountSearchFilter) ->willReturn($queryResult); - $this->accountSearchDataBuilder - ->expects(self::once()) - ->method('buildFrom'); + $out = $this->accountSearch->getByFilter($accountSearchFilter); - $this->accountSearch->getByFilter($accountSearchFilter); + $this->assertSame($queryResult, $out); } /** - * @throws QueryException - * @throws ConstraintException - * @throws SPException + * @param string $search + * @param array $expected */ #[DataProvider('searchByItemDataProvider')] public function testGetByFilterUsingItems(string $search, array $expected) @@ -100,10 +91,6 @@ class AccountSearchTest extends UnitaryTestCase ->with($accountSearchFilter) ->willReturn($queryResult); - $this->accountSearchDataBuilder - ->expects(self::once()) - ->method('buildFrom'); - $this->buildExpectationForFilter(array_keys($expected)[0]); $this->accountSearch->getByFilter($accountSearchFilter); @@ -154,9 +141,8 @@ class AccountSearchTest extends UnitaryTestCase } /** - * @throws QueryException - * @throws ConstraintException - * @throws SPException + * @param string $search + * @param array $expected */ #[DataProvider('searchByItemDataProvider')] public function testGetByFilterUsingItemsDoesNotThrowException(string $search, array $expected) @@ -170,10 +156,6 @@ class AccountSearchTest extends UnitaryTestCase ->with($accountSearchFilter) ->willReturn($queryResult); - $this->accountSearchDataBuilder - ->expects(self::once()) - ->method('buildFrom'); - $mock = $this->buildExpectationForFilter(array_keys($expected)[0]); $mock->willThrowException(new RuntimeException('test')); @@ -181,9 +163,8 @@ class AccountSearchTest extends UnitaryTestCase } /** - * @throws QueryException - * @throws ConstraintException - * @throws SPException + * @param string $search + * @param array $expected */ #[DataProvider('searchByConditionDataProvider')] public function testGetByFilterUsingConditions(string $search, array $expected) @@ -197,10 +178,6 @@ class AccountSearchTest extends UnitaryTestCase ->with($accountSearchFilter) ->willReturn($queryResult); - $this->accountSearchDataBuilder - ->expects(self::once()) - ->method('buildFrom'); - $this->buildExpectationForCondition($expected[0]); $this->accountSearch->getByFilter($accountSearchFilter); @@ -242,9 +219,9 @@ class AccountSearchTest extends UnitaryTestCase ->method('getByLogin') ->willReturn( new User([ - 'id' => self::$faker->randomNumber(), - 'userGroupId' => self::$faker->randomNumber(), - ]) + 'id' => self::$faker->randomNumber(), + 'userGroupId' => self::$faker->randomNumber(), + ]) ); $userGroupService = $this->createMock(UserGroupService::class); @@ -252,19 +229,17 @@ class AccountSearchTest extends UnitaryTestCase ->method('getByName') ->willReturn( new UserGroup([ - 'id' => self::$faker->randomNumber(), - ]) + 'id' => self::$faker->randomNumber(), + ]) ); $this->accountSearchRepository = $this->createMock(AccountSearchRepository::class); - $this->accountSearchDataBuilder = $this->createMock(AccountSearchDataBuilder::class); $this->accountSearch = new AccountSearch( $this->application, $userService, $userGroupService, - $this->accountSearchRepository, - $this->accountSearchDataBuilder + $this->accountSearchRepository ); } } diff --git a/tests/SP/Domain/Account/Services/Builders/AccountSearchDataTest.php b/tests/SP/Domain/SP/Tests/Modules/Web/Controllers/Helpers/Account/AccountSearchDataTest.php similarity index 97% rename from tests/SP/Domain/Account/Services/Builders/AccountSearchDataTest.php rename to tests/SP/Domain/SP/Tests/Modules/Web/Controllers/Helpers/Account/AccountSearchDataTest.php index f1f8671f..2d7335bf 100644 --- a/tests/SP/Domain/Account/Services/Builders/AccountSearchDataTest.php +++ b/tests/SP/Domain/SP/Tests/Modules/Web/Controllers/Helpers/Account/AccountSearchDataTest.php @@ -1,6 +1,5 @@ . */ -namespace SP\Tests\Domain\Account\Services\Builders; +declare(strict_types=1); + +namespace SP\Tests\Modules\Web\Controllers\Helpers\Account; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\MockObject\Exception; @@ -35,7 +36,6 @@ use SP\Domain\Account\Ports\AccountAclService; use SP\Domain\Account\Ports\AccountCacheService; use SP\Domain\Account\Ports\AccountToFavoriteService; use SP\Domain\Account\Ports\AccountToTagRepository; -use SP\Domain\Account\Services\Builders\AccountSearchData; use SP\Domain\Common\Models\Item; use SP\Domain\Core\Acl\AclActionsInterface; use SP\Domain\Core\Bootstrap\UriContextInterface; @@ -45,6 +45,7 @@ use SP\Domain\Core\Exceptions\SPException; use SP\Domain\Storage\Ports\FileCacheService; use SP\Infrastructure\Database\QueryResult; use SP\Infrastructure\File\FileException; +use SP\Modules\Web\Controllers\Helpers\Account\AccountSearchData; use SP\Tests\Generators\AccountDataGenerator; use SP\Tests\UnitaryTestCase; @@ -52,7 +53,7 @@ use function PHPUnit\Framework\exactly; use function PHPUnit\Framework\once; /** - * Class AccountSearchDataBuilderTest + * Class AccountSearchDataTest * */ #[Group('unitary')] @@ -168,7 +169,7 @@ class AccountSearchDataTest extends UnitaryTestCase ->willThrowException(new FileException('test')); new AccountSearchData( - $this->application, + $this->context, $this->accountAclService, $this->accountToTagRepository, $this->accountToFavoriteService, @@ -196,7 +197,7 @@ class AccountSearchDataTest extends UnitaryTestCase $this->accountSearchDataBuilder = new AccountSearchData( - $this->application, + $this->context, $this->accountAclService, $this->accountToTagRepository, $this->accountToFavoriteService, @@ -206,5 +207,4 @@ class AccountSearchDataTest extends UnitaryTestCase $uriContext ); } - }