Skip to content

Commit 72c1731

Browse files
committed
fix: align node handling for elements
Signed-off-by: Vitor Mattos <[email protected]>
1 parent 0a38f5b commit 72c1731

File tree

10 files changed

+205
-45
lines changed

10 files changed

+205
-45
lines changed

lib/Db/FileMapper.php

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -220,31 +220,42 @@ public function getFilesOfAccount(string $userId): array {
220220
return $return;
221221
}
222222

223-
public function getFileType(int $id): string {
223+
public function getDeletionContext(int $nodeId): array {
224224
$fullOuterJoin = $this->db->getQueryBuilder();
225225
$fullOuterJoin->select($fullOuterJoin->expr()->literal(1));
226226

227227
$qb = $this->db->getQueryBuilder();
228228
$qb
229-
->selectAlias('f.id', 'file')
230-
->selectAlias('sf.signed_node_id', 'signed_file')
231-
->selectAlias('ue.id', 'user_element')
232-
->selectAlias('fe.id', 'file_element')
229+
->selectAlias('f.id', 'file_id')
230+
->addSelect('sf.id', 'signed_file_id')
231+
->addSelect('ue.id', 'user_element_id')
232+
->addSelect('fe.file_id', 'file_element_file_id')
233233
->from($qb->createFunction('(' . $fullOuterJoin->getSQL() . ')'), 'foj')
234-
->leftJoin('foj', 'libresign_file', 'f', $qb->expr()->eq('f.node_id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)))
235-
->leftJoin('foj', 'libresign_file', 'sf', $qb->expr()->eq('sf.signed_node_id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)))
236-
->leftJoin('foj', 'libresign_user_element', 'ue', $qb->expr()->eq('ue.file_id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)))
237-
->leftJoin('foj', 'libresign_file_element', 'fe', $qb->expr()->eq('fe.file_id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)));
238-
$cursor = $qb->executeQuery();
239-
$row = $cursor->fetch();
240-
if ($row) {
241-
foreach ($row as $key => $value) {
242-
if ($value) {
243-
return $key;
244-
}
245-
}
234+
->leftJoin('foj', 'libresign_file', 'f', $qb->expr()->eq('f.node_id', $qb->createNamedParameter($nodeId, IQueryBuilder::PARAM_INT)))
235+
->leftJoin('foj', 'libresign_file', 'sf', $qb->expr()->eq('sf.signed_node_id', $qb->createNamedParameter($nodeId, IQueryBuilder::PARAM_INT)))
236+
->leftJoin('foj', 'libresign_user_element', 'ue', $qb->expr()->eq('ue.node_id', $qb->createNamedParameter($nodeId, IQueryBuilder::PARAM_INT)))
237+
->leftJoin('foj', 'libresign_file_element', 'fe', $qb->expr()->eq('fe.file_id', 'f.id'))
238+
->setMaxResults(1);
239+
240+
$row = $qb->executeQuery()->fetchAssociative();
241+
if (!$row) {
242+
return ['type' => 'not_libresign_file', 'fileId' => null];
246243
}
247-
return 'not_libresign_file';
244+
245+
if (!empty($row['signed_file_id'])) {
246+
return ['type' => 'signed_file', 'fileId' => (int) $row['signed_file_id']];
247+
}
248+
if (!empty($row['file_id'])) {
249+
return ['type' => 'file', 'fileId' => (int) $row['file_id']];
250+
}
251+
if (!empty($row['user_element_id'])) {
252+
return ['type' => 'user_element', 'fileId' => null];
253+
}
254+
if (!empty($row['file_element_file_id'])) {
255+
return ['type' => 'file_element', 'fileId' => (int) $row['file_element_file_id']];
256+
}
257+
258+
return ['type' => 'not_libresign_file', 'fileId' => null];
248259
}
249260

250261
public function getTextOfStatus(int|FileStatus $status): string {

lib/Db/UserElement.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
* @method int getId()
1717
* @method void setType(string $type)
1818
* @method string getType()
19-
* @method void setFileId(int $fileId)
20-
* @method int getFileId()
19+
* @method void setNodeId(int $nodeId)
20+
* @method int getNodeId()
2121
* @method void setUserId(string $userId)
2222
* @method void setStarred(int $starred)
2323
* @method int getStarred()
@@ -27,7 +27,7 @@
2727
*/
2828
class UserElement extends Entity {
2929
public string $type = '';
30-
protected int $fileId = 0;
30+
protected int $nodeId = 0;
3131
protected string $userId = '';
3232
public bool $starred = false;
3333
public ?\DateTime $createdAt = null;
@@ -37,7 +37,7 @@ class UserElement extends Entity {
3737
public function __construct() {
3838
$this->addType('id', Types::INTEGER);
3939
$this->addType('type', Types::STRING);
40-
$this->addType('fileId', Types::INTEGER);
40+
$this->addType('nodeId', Types::INTEGER);
4141
$this->addType('userId', Types::STRING);
4242
$this->addType('starred', Types::INTEGER);
4343
$this->addType('createdAt', Types::DATETIME);

lib/Db/UserElementMapper.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ private function getQueryBuilder(array $data): IQueryBuilder {
3434
$qb->expr()->eq('ue.id', $qb->createNamedParameter($data['id'], IQueryBuilder::PARAM_INT))
3535
);
3636
}
37-
if (isset($data['file_id'])) {
37+
if (isset($data['node_id'])) {
3838
$qb->andWhere(
39-
$qb->expr()->eq('ue.file_id', $qb->createNamedParameter($data['file_id'], IQueryBuilder::PARAM_INT))
39+
$qb->expr()->eq('ue.node_id', $qb->createNamedParameter($data['node_id'], IQueryBuilder::PARAM_INT))
4040
);
4141
}
4242
if (isset($data['type'])) {

lib/Helper/ValidateHelper.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ public function validateVisibleElementsRelation(array $list, SignRequest $signRe
302302
$this->validateSignerIsOwnerOfPdfVisibleElement($elements['documentElementId'], $signRequest);
303303
if ($canCreateSignature && $user instanceof IUser) {
304304
try {
305-
$this->userElementMapper->findOne(['file_id' => $elements['profileNodeId'], 'user_id' => $user->getUID()]);
305+
$this->userElementMapper->findOne(['node_id' => $elements['profileNodeId'], 'user_id' => $user->getUID()]);
306306
} catch (\Throwable) {
307307
throw new LibresignException($this->l10n->t('Field %s does not belong to user', $elements['profileNodeId']));
308308
}

lib/Listener/BeforeNodeDeletedListener.php

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,26 +51,35 @@ public function handle(Event $event): void {
5151
}
5252

5353
private function delete(int $nodeId): void {
54-
$type = $this->fileMapper->getFileType($nodeId);
55-
if ($type === 'not_libresign_file') {
54+
$context = $this->fileMapper->getDeletionContext($nodeId);
55+
if ($context['type'] === 'not_libresign_file') {
5656
return;
5757
}
58-
switch ($type) {
58+
switch ($context['type']) {
5959
case 'signed_file':
60-
$file = $this->fileMapper->getByNodeId($nodeId);
61-
$this->requestSignatureService->deleteRequestSignature(['file' => ['fileId' => $file->getId()]]);
60+
if (!isset($context['fileId'])) {
61+
return;
62+
}
63+
$this->requestSignatureService->deleteRequestSignature(['file' => ['fileId' => $context['fileId']]]);
6264
break;
6365
case 'file':
6466
$libresignFile = $this->fileMapper->getByNodeId($nodeId);
6567
$this->requestSignatureService->deleteRequestSignature(['file' => ['fileId' => $libresignFile->getId()]]);
6668
$this->fileMapper->delete($libresignFile);
6769
break;
6870
case 'user_element':
71+
$qb = $this->db->getQueryBuilder();
72+
$qb->delete('libresign_user_element')
73+
->where($qb->expr()->eq('node_id', $qb->createNamedParameter($nodeId, IQueryBuilder::PARAM_INT)))
74+
->executeStatement();
75+
break;
6976
case 'file_element':
70-
$field = $type === 'file' ? 'node_id' : 'file_id';
77+
if (!isset($context['fileId'])) {
78+
return;
79+
}
7180
$qb = $this->db->getQueryBuilder();
72-
$qb->delete('libresign_' . $type)
73-
->where($qb->expr()->eq($field, $qb->createNamedParameter($nodeId, IQueryBuilder::PARAM_INT)))
81+
$qb->delete('libresign_file_element')
82+
->where($qb->expr()->eq('file_id', $qb->createNamedParameter($context['fileId'], IQueryBuilder::PARAM_INT)))
7483
->executeStatement();
7584
}
7685
}
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2025 LibreCode coop and contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Libresign\Migration;
10+
11+
use Closure;
12+
use OCP\DB\ISchemaWrapper;
13+
use OCP\DB\QueryBuilder\IQueryBuilder;
14+
use OCP\DB\Types;
15+
use OCP\Files\AppData\IAppDataFactory;
16+
use OCP\Files\IAppData;
17+
use OCP\IDBConnection;
18+
use OCP\Migration\IOutput;
19+
use OCP\Migration\SimpleMigrationStep;
20+
21+
class Version16002Date20251230120000 extends SimpleMigrationStep {
22+
protected IAppData $appData;
23+
24+
public function __construct(
25+
private IDBConnection $connection,
26+
private IAppDataFactory $appDataFactory,
27+
) {
28+
$this->appData = $appDataFactory->get('libresign');
29+
}
30+
31+
#[\Override]
32+
public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
33+
$this->backupUserElementTable();
34+
}
35+
36+
#[\Override]
37+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
38+
/** @var ISchemaWrapper $schema */
39+
$schema = $schemaClosure();
40+
$changed = false;
41+
42+
if ($schema->hasTable('libresign_user_element')) {
43+
$table = $schema->getTable('libresign_user_element');
44+
if (!$table->hasColumn('node_id')) {
45+
$table->addColumn('node_id', Types::BIGINT, [
46+
'notnull' => false,
47+
'unsigned' => true,
48+
]);
49+
$changed = true;
50+
}
51+
52+
if ($table->hasColumn('file_id')) {
53+
$table->dropColumn('file_id');
54+
$changed = true;
55+
}
56+
}
57+
58+
return $changed ? $schema : null;
59+
}
60+
61+
private function backupUserElementTable(): void {
62+
$qb = $this->connection->getQueryBuilder();
63+
$qb->select('*')
64+
->from('libresign_user_element')
65+
->orderBy('id');
66+
67+
$cursor = $qb->executeQuery();
68+
$row = $cursor->fetch();
69+
if (!$row) {
70+
$cursor->closeCursor();
71+
return;
72+
}
73+
74+
$folder = $this->appData->getFolder('/');
75+
$file = $folder->newFile('backup-table-libresign_user_element_Version16002Date20251230120000.csv');
76+
$fp = $file->write();
77+
78+
fputcsv($fp, array_keys($row));
79+
fputcsv($fp, $row);
80+
while ($row = $cursor->fetch()) {
81+
fputcsv($fp, $row);
82+
}
83+
84+
fclose($fp);
85+
$cursor->closeCursor();
86+
}
87+
88+
private function restoreNodeIdsFromBackup(): void {
89+
$folder = $this->appData->getFolder('/');
90+
$filename = 'backup-table-libresign_user_element_Version16002Date20251230120000.csv';
91+
if (!$folder->fileExists($filename)) {
92+
return;
93+
}
94+
95+
$file = $folder->getFile($filename);
96+
$handle = $file->read();
97+
if ($handle === false) {
98+
return;
99+
}
100+
101+
$header = fgetcsv($handle);
102+
if ($header === false) {
103+
fclose($handle);
104+
return;
105+
}
106+
107+
$columnIndex = array_flip($header);
108+
if (!isset($columnIndex['id']) || !isset($columnIndex['file_id'])) {
109+
fclose($handle);
110+
return;
111+
}
112+
113+
while (($row = fgetcsv($handle)) !== false) {
114+
if (!isset($row[$columnIndex['id']])) {
115+
continue;
116+
}
117+
118+
$userElementId = (int) $row[$columnIndex['id']];
119+
$nodeId = $row[$columnIndex['file_id']] ?? null;
120+
if ($nodeId === null || $nodeId === '') {
121+
continue;
122+
}
123+
124+
$qb = $this->connection->getQueryBuilder();
125+
$qb->update('libresign_user_element')
126+
->set('node_id', $qb->createNamedParameter((int) $nodeId, IQueryBuilder::PARAM_INT))
127+
->where($qb->expr()->eq('id', $qb->createNamedParameter($userElementId, IQueryBuilder::PARAM_INT)));
128+
129+
$qb->executeStatement();
130+
}
131+
132+
fclose($handle);
133+
}
134+
135+
#[\Override]
136+
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
137+
$this->restoreNodeIdsFromBackup();
138+
}
139+
140+
}

lib/Service/AccountService.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ private function updateFileOfVisibleElement(array $data): void {
394394
return;
395395
}
396396
$userElement = $this->userElementMapper->findOne(['id' => $data['elementId']]);
397-
$file = $this->folderService->getFileByNodeId($userElement->getFileId());
397+
$file = $this->folderService->getFileByNodeId($userElement->getNodeId());
398398
$file->putContent($this->getFileRaw($data));
399399
}
400400

@@ -449,7 +449,7 @@ private function createFileOfVisibleElementUsingSession(array $data, string $ses
449449
private function insertVisibleElement(array $data, IUser $user, File $file): void {
450450
$userElement = new UserElement();
451451
$userElement->setType($data['type']);
452-
$userElement->setFileId($file->getId());
452+
$userElement->setNodeId($file->getId());
453453
$userElement->setUserId($user->getUID());
454454
$userElement->setStarred(isset($data['starred']) && $data['starred'] ? 1 : 0);
455455
$userElement->setCreatedAt($this->timeFactory->getDateTime());
@@ -489,12 +489,12 @@ private function getFileRaw(array $data): string {
489489
public function deleteSignatureElement(?IUser $user, string $sessionId, int $nodeId): void {
490490
if ($user instanceof IUser) {
491491
$element = $this->userElementMapper->findOne([
492-
'file_id' => $nodeId,
492+
'node_id' => $nodeId,
493493
'user_id' => $user->getUID(),
494494
]);
495495
$this->userElementMapper->delete($element);
496496
try {
497-
$file = $this->folderService->getFileByNodeId($element->getFileId());
497+
$file = $this->folderService->getFileByNodeId($element->getNodeId());
498498
$file->delete();
499499
} catch (NotFoundException) {
500500
}

lib/Service/SignFileService.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ private function retrieveUserElement(FileElement $fileElement): int {
267267
} catch (MultipleObjectsReturnedException|DoesNotExistException|Exception) {
268268
throw new LibresignException($this->l10n->t('You need to define a visible signature or initials to sign this document.'));
269269
}
270-
return $userElement->getFileId();
270+
return $userElement->getNodeId();
271271
}
272272

273273
private function bindFileElementWithTempFile(FileElement $fileElement, int $nodeId): VisibleElementAssoc {

lib/Service/SignerElementsService.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public function __construct(
3737
* @return LibresignUserElement
3838
*/
3939
public function getUserElementByNodeId(string $userId, int $nodeId): array {
40-
$element = $this->userElementMapper->findOne(['file_id' => $nodeId, 'user_id' => $userId]);
40+
$element = $this->userElementMapper->findOne(['node_id' => $nodeId, 'user_id' => $userId]);
4141
$exists = $this->signatureFileExists($element);
4242
if (!$exists) {
4343
throw new NotFoundException();
@@ -48,9 +48,9 @@ public function getUserElementByNodeId(string $userId, int $nodeId): array {
4848
'file' => [
4949
'url' => $this->urlGenerator->linkToRoute('ocs.libresign.SignatureElements.getSignatureElementPreview', [
5050
'apiVersion' => 'v1',
51-
'nodeId' => $element->getFileId(),
51+
'nodeId' => $element->getNodeId(),
5252
]),
53-
'nodeId' => $element->getFileId()
53+
'nodeId' => $element->getNodeId()
5454
],
5555
'userId' => $element->getUserId(),
5656
'starred' => $element->getStarred() ? 1 : 0,
@@ -75,9 +75,9 @@ public function getUserElements(string $userId): array {
7575
'file' => [
7676
'url' => $this->urlGenerator->linkToRoute('ocs.libresign.SignatureElements.getSignatureElementPreview', [
7777
'apiVersion' => 'v1',
78-
'nodeId' => $element->getFileId(),
78+
'nodeId' => $element->getNodeId(),
7979
]),
80-
'nodeId' => $element->getFileId()
80+
'nodeId' => $element->getNodeId()
8181
],
8282
'starred' => $element->getStarred() ? 1 : 0,
8383
'userId' => $element->getUserId(),
@@ -89,7 +89,7 @@ public function getUserElements(string $userId): array {
8989

9090
private function signatureFileExists(UserElement $userElement): bool {
9191
try {
92-
$this->folderService->getFileByNodeId($userElement->getFileId());
92+
$this->folderService->getFileByNodeId($userElement->getNodeId());
9393
} catch (\Exception) {
9494
$this->userElementMapper->delete($userElement);
9595
return false;

tests/php/Unit/Service/SignFileServiceTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1029,7 +1029,7 @@ public function testSetVisibleElements(
10291029
throw new DoesNotExistException('User element not found');
10301030
});
10311031

1032-
$this->folderService->method('getFileById')
1032+
$this->folderService->method('getFileByNodeId')
10331033
->willReturnCallback(function ($id) use ($signatureFile) {
10341034
if (isset($signatureFile[$id]) && $signatureFile[$id]['valid']) {
10351035
$file = $this->getMockBuilder(\OCP\Files\File::class)->getMock();

0 commit comments

Comments
 (0)