From dd702d45cf9a1ac27e0bfbacf95322a87047ad50 Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Fri, 29 Nov 2024 09:04:39 -0300 Subject: [PATCH] fix: list files from PostgreSQL Signed-off-by: Vitor Mattos [skip ci] --- lib/Db/AccountFileMapper.php | 89 +++++++++++-------- lib/Db/PagerFantaQueryAdapter.php | 23 +---- lib/Db/SignRequestMapper.php | 74 +++++++-------- lib/Helper/Pagination.php | 3 +- lib/ResponseDefinitions.php | 6 +- lib/Service/FileService.php | 4 +- openapi-full.json | 12 +-- openapi.json | 12 +-- src/store/files.js | 4 +- src/types/openapi/openapi-full.ts | 6 +- src/types/openapi/openapi.ts | 6 +- src/views/FilesList/FileEntry/FileEntry.vue | 2 +- .../FilesList/FileEntry/FileEntryGrid.vue | 2 +- .../FilesList/FileEntry/FileEntryMixin.js | 2 +- 14 files changed, 124 insertions(+), 121 deletions(-) diff --git a/lib/Db/AccountFileMapper.php b/lib/Db/AccountFileMapper.php index 192ec131b3..d1d20f5db8 100644 --- a/lib/Db/AccountFileMapper.php +++ b/lib/Db/AccountFileMapper.php @@ -115,36 +115,48 @@ public function accountFileList(array $filter, ?int $page = null, ?int $length = return $return; } - private function getUserAccountFile(array $filter = []): Pagination { + private function getQueryBuilder(array $filter = [], bool $count = false): IQueryBuilder { $qb = $this->db->getQueryBuilder(); - $qb->select( - 'f.id', - 'f.uuid', - 'f.name', - 'f.callback', - 'f.status', - 'f.node_id', - 'af.file_type' - ) - ->selectAlias('u.uid_lower', 'account_uid') - ->selectAlias('u.displayname', 'account_displayname') - ->selectAlias('f.created_at', 'request_date') + if ($count) { + $qb->select($qb->func()->count()) + ->setFirstResult(0) + ->setMaxResults(null); + } else { + $qb + ->select( + 'f.id', + 'f.uuid', + 'f.name', + 'f.callback', + 'f.status', + 'f.node_id', + 'af.file_type', + 'f.created_at', + ) + ->selectAlias('u.uid_lower', 'account_uid') + ->selectAlias('u.displayname', 'account_displayname') + ->groupBy( + 'f.id', + 'f.uuid', + 'f.name', + 'f.callback', + 'f.status', + 'f.node_id', + 'f.created_at', + 'af.file_type', + 'u.uid_lower', + 'u.displayname' + ); + if (isset($filter['length']) && isset($filter['page'])) { + $qb->setFirstResult($filter['length'] * ($filter['page'] - 1)); + $qb->setMaxResults($filter['length']); + } + } + $qb ->from($this->getTableName(), 'af') ->join('af', 'libresign_file', 'f', 'f.id = af.file_id') ->join('af', 'users', 'u', 'af.user_id = u.uid') - ->leftJoin('f', 'libresign_sign_request', 'sr', 'sr.file_id = f.id') - ->groupBy( - 'f.id', - 'f.uuid', - 'f.name', - 'f.callback', - 'f.status', - 'f.node_id', - 'f.created_at', - 'af.file_type', - 'u.uid_lower', - 'u.displayname' - ); + ->leftJoin('f', 'libresign_sign_request', 'sr', 'sr.file_id = f.id'); if (!empty($filter['userId'])) { $qb->where( $qb->expr()->eq('af.user_id', $qb->createNamedParameter($filter['userId'])), @@ -162,12 +174,19 @@ private function getUserAccountFile(array $filter = []): Pagination { $qb->expr()->eq('af.user_id', $qb->createNamedParameter($filter['userId'])), ); } - if (isset($filter['length']) && isset($filter['page'])) { - $qb->setFirstResult($filter['length'] * ($filter['page'] - 1)); - $qb->setMaxResults($filter['length']); - } + return $qb; + } + + private function getUserAccountFile(array $filter = []): Pagination { + $qb = $this->getQueryBuilder( + filter: $filter, + ); + $countQb = $this->getQueryBuilder( + filter: $filter, + count: true, + ); - $pagination = new Pagination($qb, $this->urlGenerator); + $pagination = new Pagination($qb, $this->urlGenerator, $countQb); return $pagination; } @@ -181,14 +200,14 @@ private function formatListRow(array $row, string $url): array { 'name' => $this->fileTypeMapper->getNameOfType($row['file_type']), 'description' => $this->fileTypeMapper->getDescriptionOfType($row['file_type']), ]; - $row['request_date'] = (new \DateTime()) - ->setTimestamp((int)$row['request_date']) + $row['created_at'] = (new \DateTime()) + ->setTimestamp((int)$row['created_at']) ->format('Y-m-d H:i:s'); $row['file'] = [ 'name' => $row['name'], 'status' => $row['status'], 'statusText' => $this->fileMapper->getTextOfStatus((int)$row['status']), - 'request_date' => $row['request_date'], + 'created_at' => $row['created_at'], 'file' => [ 'type' => 'pdf', 'nodeId' => (int)$row['node_id'], @@ -201,7 +220,7 @@ private function formatListRow(array $row, string $url): array { $row['node_id'], $row['name'], $row['status'], - $row['request_date'], + $row['created_at'], $row['account_displayname'], $row['account_uid'], $row['callback'], diff --git a/lib/Db/PagerFantaQueryAdapter.php b/lib/Db/PagerFantaQueryAdapter.php index ce913d49ac..bbe18a1b45 100644 --- a/lib/Db/PagerFantaQueryAdapter.php +++ b/lib/Db/PagerFantaQueryAdapter.php @@ -12,7 +12,6 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use Pagerfanta\Adapter\AdapterInterface; use Pagerfanta\Exception\InvalidArgumentException; -use ReflectionClass; /** * Adapter which calculates pagination from a Doctrine DBAL QueryBuilder. @@ -23,6 +22,7 @@ class PagerFantaQueryAdapter implements AdapterInterface { */ public function __construct( private IQueryBuilder $queryBuilder, + private IQueryBuilder $countQueryBuilder, ) { if ($queryBuilder->getType() !== QueryBuilder::SELECT) { // @codeCoverageIgnoreStart @@ -32,26 +32,7 @@ public function __construct( } public function getNbResults(): int { - /** - * The clone isn't working fine if we clone the property $this->queryBuilder - * because the internal property "queryBuilder" of $this->queryBuilder is - * a reference and the clone don't work with reference. To solve this - * was used reflection. - */ - $reflect = new ReflectionClass($this->queryBuilder); - $reflectionProperty = $reflect->getProperty('queryBuilder'); - $reflectionProperty->setAccessible(true); - $qb = $reflectionProperty->getValue($this->queryBuilder); - $originalQueryBuilder = clone $qb; - - $this->queryBuilder->resetQueryPart('select') - ->resetQueryPart('groupBy') - ->select($this->queryBuilder->func()->count()) - ->setFirstResult(0) - ->setMaxResults(null); - $total = $this->queryBuilder->executeQuery()->fetchOne(); - - $reflectionProperty->setValue($this->queryBuilder, $originalQueryBuilder); + $total = $this->countQueryBuilder->executeQuery()->fetchOne(); return abs((int)$total); } diff --git a/lib/Db/SignRequestMapper.php b/lib/Db/SignRequestMapper.php index 37517ae136..3e2299fb71 100644 --- a/lib/Db/SignRequestMapper.php +++ b/lib/Db/SignRequestMapper.php @@ -446,7 +446,6 @@ public function getMyLibresignFile(string $userId, ?array $filter = []): File { userId: $userId, filter: $filter, ); - $qb->select('f.*'); $cursor = $qb->executeQuery(); $row = $cursor->fetch(); if (!$row) { @@ -456,26 +455,46 @@ public function getMyLibresignFile(string $userId, ?array $filter = []): File { return $file->fromRow($row); } - private function getFilesAssociatedFilesWithMeQueryBuilder(string $userId, ?array $filter = []): IQueryBuilder { + private function getFilesAssociatedFilesWithMeQueryBuilder(string $userId, array $filter = [], bool $count = false): IQueryBuilder { $qb = $this->db->getQueryBuilder(); $qb->from('libresign_file', 'f') ->leftJoin('f', 'libresign_sign_request', 'sr', 'sr.file_id = f.id') - ->leftJoin('f', 'libresign_identify_method', 'im', $qb->expr()->eq('sr.id', 'im.sign_request_id')) - ->groupBy( + ->leftJoin('f', 'libresign_identify_method', 'im', $qb->expr()->eq('sr.id', 'im.sign_request_id')); + if ($count) { + $qb->select($qb->func()->count()) + ->setFirstResult(0) + ->setMaxResults(null); + } else { + $qb->select( 'f.id', 'f.node_id', 'f.user_id', 'f.uuid', 'f.name', 'f.status', + 'f.metadata', 'f.created_at', - ); - // metadata is a json column, the right way is to use f.metadata::text - // when the database is PostgreSQL. The problem is that the command - // addGroupBy add quotes over all text send as argument. With - // PostgreSQL json columns don't have problem if not added to group by. - if ($qb->getConnection()->getDatabaseProvider() !== IDBConnection::PLATFORM_POSTGRES) { - $qb->addGroupBy('f.metadata'); + ) + ->groupBy( + 'f.id', + 'f.node_id', + 'f.user_id', + 'f.uuid', + 'f.name', + 'f.status', + 'f.created_at', + ); + // metadata is a json column, the right way is to use f.metadata::text + // when the database is PostgreSQL. The problem is that the command + // addGroupBy add quotes over all text send as argument. With + // PostgreSQL json columns don't have problem if not added to group by. + if ($qb->getConnection()->getDatabaseProvider() !== IDBConnection::PLATFORM_POSTGRES) { + $qb->addGroupBy('f.metadata'); + } + if (isset($filter['length']) && isset($filter['page'])) { + $qb->setFirstResult($filter['length'] * ($filter['page'] - 1)); + $qb->setMaxResults($filter['length']); + } } $or = [ @@ -518,10 +537,6 @@ private function getFilesAssociatedFilesWithMeQueryBuilder(string $userId, ?arra $qb->expr()->lte('f.created_at', $qb->createNamedParameter($filter['end'], IQueryBuilder::PARAM_INT)) ); } - if (isset($filter['length']) && isset($filter['page'])) { - $qb->setFirstResult($filter['length'] * ($filter['page'] - 1)); - $qb->setMaxResults($filter['length']); - } } return $qb; } @@ -532,33 +547,20 @@ private function getFilesAssociatedFilesWithMeStmt( ?array $sort = [], ): Pagination { $qb = $this->getFilesAssociatedFilesWithMeQueryBuilder($userId, $filter); - $qb->select( - 'f.id', - 'f.node_id', - 'f.user_id', - 'f.uuid', - 'f.name', - 'f.status', - 'f.metadata', - ); if (!empty($sort) && in_array($sort['sortBy'], ['name', 'status', 'created_at'])) { $qb->orderBy( $qb->func()->lower('f.' . $sort['sortBy']), $sort['sortDirection'] == 'asc' ? 'asc' : 'desc' ); } - $qb->selectAlias('f.created_at', 'request_date'); - $countQueryBuilderModifier = function (IQueryBuilder $qb): int { - $qb->resetQueryPart('select') - ->resetQueryPart('groupBy') - ->select($qb->func()->count()) - ->setFirstResult(0) - ->setMaxResults(null); - return (int)$qb->executeQuery()->fetchOne(); - }; + $countQb = $this->getFilesAssociatedFilesWithMeQueryBuilder( + userId: $userId, + filter: $filter, + count: true, + ); - $pagination = new Pagination($qb, $this->urlGenerator); + $pagination = new Pagination($qb, $this->urlGenerator, $countQb); return $pagination; } @@ -571,8 +573,8 @@ private function formatListRow(array $row): array { 'userId' => $row['user_id'], 'displayName' => $this->userManager->get($row['user_id'])?->getDisplayName(), ]; - $row['request_date'] = (new \DateTime()) - ->setTimestamp((int)$row['request_date']) + $row['created_at'] = (new \DateTime()) + ->setTimestamp((int)$row['created_at']) ->format('Y-m-d H:i:s'); $row['file'] = $this->urlGenerator->linkToRoute('libresign.page.getPdf', ['uuid' => $row['uuid']]); $row['nodeId'] = (int)$row['node_id']; diff --git a/lib/Helper/Pagination.php b/lib/Helper/Pagination.php index 5e1f87db8f..d15cfe5a8b 100644 --- a/lib/Helper/Pagination.php +++ b/lib/Helper/Pagination.php @@ -34,8 +34,9 @@ class Pagination extends Pagerfanta { public function __construct( IQueryBuilder $queryBuilder, private IURLGenerator $urlGenerator, + private IQueryBuilder $countQueryBuilder, ) { - $adapter = new PagerFantaQueryAdapter($queryBuilder); + $adapter = new PagerFantaQueryAdapter($queryBuilder, $countQueryBuilder); parent::__construct($adapter); } diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 2b0756e76e..cd22332e41 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -140,7 +140,7 @@ * status: 0|1|2|3|4, * statusText: string, * nodeId: non-negative-int, - * request_date: string, + * created_at: string, * requested_by: array{ * userId: string, * displayName: string, @@ -165,12 +165,12 @@ * name: string, * description: ?string, * }, - * request_date: string, + * created_at: string, * file: array{ * name: string, * status: 0|1|2|3|4, * statusText: string, - * request_date: string, + * created_at: string, * file: array{ * type: string, * nodeId: non-negative-int, diff --git a/lib/Service/FileService.php b/lib/Service/FileService.php index caf52aa0fc..9c5b249a24 100644 --- a/lib/Service/FileService.php +++ b/lib/Service/FileService.php @@ -375,7 +375,7 @@ private function getFile(): array { $return['uuid'] = $this->file->getUuid(); $return['name'] = $this->file->getName(); $return['status'] = $this->file->getStatus(); - $return['request_date'] = (new \DateTime()) + $return['created_at'] = (new \DateTime()) ->setTimestamp($this->file->getCreatedAt()) ->format('Y-m-d H:i:s'); $return['statusText'] = $this->fileMapper->getTextOfStatus($this->file->getStatus()); @@ -535,7 +535,7 @@ private function associateAllAndFormat(IUser $user, array $files, array $signers }, false), 'visibleElements' => $this->formatVisibleElementsToArray( $visibleElements[$signer->getId()] ?? [], - json_decode($file['metadata'], true) + !empty($file['metadata'])?json_decode($file['metadata'], true):[] ), 'identifyMethods' => array_map(function (IdentifyMethod $identifyMethod) use ($signer): array { return [ diff --git a/openapi-full.json b/openapi-full.json index 8479e0bb33..5ddd9228d2 100644 --- a/openapi-full.json +++ b/openapi-full.json @@ -194,7 +194,7 @@ "required": [ "account", "file_type", - "request_date", + "created_at", "file" ], "properties": { @@ -233,7 +233,7 @@ } } }, - "request_date": { + "created_at": { "type": "string" }, "file": { @@ -242,7 +242,7 @@ "name", "status", "statusText", - "request_date", + "created_at", "file", "callback", "uuid", @@ -266,7 +266,7 @@ "statusText": { "type": "string" }, - "request_date": { + "created_at": { "type": "string" }, "file": { @@ -711,7 +711,7 @@ "status", "statusText", "nodeId", - "request_date", + "created_at", "requested_by", "file" ], @@ -741,7 +741,7 @@ "format": "int64", "minimum": 0 }, - "request_date": { + "created_at": { "type": "string" }, "requested_by": { diff --git a/openapi.json b/openapi.json index 5e52209878..57c8b590c9 100644 --- a/openapi.json +++ b/openapi.json @@ -131,7 +131,7 @@ "required": [ "account", "file_type", - "request_date", + "created_at", "file" ], "properties": { @@ -170,7 +170,7 @@ } } }, - "request_date": { + "created_at": { "type": "string" }, "file": { @@ -179,7 +179,7 @@ "name", "status", "statusText", - "request_date", + "created_at", "file", "callback", "uuid", @@ -203,7 +203,7 @@ "statusText": { "type": "string" }, - "request_date": { + "created_at": { "type": "string" }, "file": { @@ -615,7 +615,7 @@ "status", "statusText", "nodeId", - "request_date", + "created_at", "requested_by", "file" ], @@ -645,7 +645,7 @@ "format": "int64", "minimum": 0 }, - "request_date": { + "created_at": { "type": "string" }, "requested_by": { diff --git a/src/store/files.js b/src/store/files.js index 14b80ad205..f3175be076 100644 --- a/src/store/files.js +++ b/src/store/files.js @@ -159,12 +159,12 @@ export const useFilesStore = function(...args) { return '' } const file = this.getFile() - if ((file?.requested_by?.userId ?? '').length === 0 || file?.request_date.length === 0) { + if ((file?.requested_by?.userId ?? '').length === 0 || file?.created_at.length === 0) { return '' } return t('libresign', 'Requested by {name}, at {date}', { name: file.requested_by.userId, - date: Moment(Date.parse(file.request_date)).format('LL LTS'), + date: Moment(Date.parse(file.created_at)).format('LL LTS'), }) }, async hydrateFile(nodeId) { diff --git a/src/types/openapi/openapi-full.ts b/src/types/openapi/openapi-full.ts index bec8581075..b056671a74 100644 --- a/src/types/openapi/openapi-full.ts +++ b/src/types/openapi/openapi-full.ts @@ -1101,7 +1101,7 @@ export type components = { name: string; description: string | null; }; - request_date: string; + created_at: string; file: { name: string; /** @@ -1110,7 +1110,7 @@ export type components = { */ status: 0 | 1 | 2 | 3 | 4; statusText: string; - request_date: string; + created_at: string; file: { type: string; /** Format: int64 */ @@ -1248,7 +1248,7 @@ export type components = { statusText: string; /** Format: int64 */ nodeId: number; - request_date: string; + created_at: string; requested_by: { userId: string; displayName: string; diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index 04c7b08487..a1aa3434ee 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -963,7 +963,7 @@ export type components = { name: string; description: string | null; }; - request_date: string; + created_at: string; file: { name: string; /** @@ -972,7 +972,7 @@ export type components = { */ status: 0 | 1 | 2 | 3 | 4; statusText: string; - request_date: string; + created_at: string; file: { type: string; /** Format: int64 */ @@ -1102,7 +1102,7 @@ export type components = { statusText: string; /** Format: int64 */ nodeId: number; - request_date: string; + created_at: string; requested_by: { userId: string; displayName: string; diff --git a/src/views/FilesList/FileEntry/FileEntry.vue b/src/views/FilesList/FileEntry/FileEntry.vue index 69e4c57e2f..688d7b0379 100644 --- a/src/views/FilesList/FileEntry/FileEntry.vue +++ b/src/views/FilesList/FileEntry/FileEntry.vue @@ -35,7 +35,7 @@ - + diff --git a/src/views/FilesList/FileEntry/FileEntryGrid.vue b/src/views/FilesList/FileEntry/FileEntryGrid.vue index cb0006c97e..4ac1576d24 100644 --- a/src/views/FilesList/FileEntry/FileEntryGrid.vue +++ b/src/views/FilesList/FileEntry/FileEntryGrid.vue @@ -28,7 +28,7 @@ - + diff --git a/src/views/FilesList/FileEntry/FileEntryMixin.js b/src/views/FilesList/FileEntry/FileEntryMixin.js index 8cbcbd6985..a5932bfdf1 100644 --- a/src/views/FilesList/FileEntry/FileEntryMixin.js +++ b/src/views/FilesList/FileEntry/FileEntryMixin.js @@ -16,7 +16,7 @@ export default { }, computed: { mtime() { - return Date.parse(this?.source?.request_date) + return Date.parse(this?.source?.created_at) }, openedMenu: {