Skip to content

Commit 3333b1e

Browse files
max-nextcloudmax-nextcloud
committed
chore(refactor): simplify IContext API with helper classes
For some reason `$document->getId` is suspected to return void by vscode. `$document->id` is public. So use that instead. Signed-off-by: Max <max@nextcloud.com>
1 parent ff8db14 commit 3333b1e

8 files changed

Lines changed: 217 additions & 133 deletions

File tree

‎lib/Context/FileContext.php‎

Lines changed: 62 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,63 +14,107 @@
1414
use OCP\Files\Lock\ILock;
1515
use OCP\IL10N;
1616
use OCP\IUser;
17+
use Override;
18+
use Psr\Log\LoggerInterface;
1719

1820
class FileContext implements IContext {
1921

2022
public function __construct(
2123
private readonly FileService $fileService,
2224
private readonly IL10N $l10n,
2325
private readonly LockService $lockService,
26+
private readonly LoggerInterface $logger,
2427
private readonly File $file,
2528
private readonly ?string $baseVersionEtag,
2629
private readonly ?string $token = null,
2730
) {
2831
}
2932

30-
public function check(): ?string {
33+
#[Override]
34+
public function getId(): int {
35+
return $this->file->getId();
36+
}
37+
38+
#[Override]
39+
public function getType(): string {
40+
return 'file';
41+
}
42+
43+
#[Override]
44+
public function toString(): string {
45+
return $this->getType() . ' (' . $this->getId() . ')';
46+
}
47+
48+
#[Override]
49+
public function buildDocument(): Document|string {
3150
// Block using text for disabled download internal shares
3251
if ($this->fileService->isDownloadDisabled($this->file)) {
3352
return $this->l10n->t('This file cannot be displayed as download is disabled by the share');
3453
}
35-
return null;
54+
$document = new Document();
55+
$document->setId($this->getId());
56+
$document->setLastSavedVersion(0);
57+
$document->setLastSavedVersionTime($this->file->getMTime());
58+
$document->setLastSavedVersionEtag($this->file->getEtag());
59+
$document->setChecksum($this->computeChecksum());
60+
// This is a new document - so it needs a fresh base version etag.
61+
$document->setBaseVersionEtag(uniqid());
62+
return $document;
3663
}
3764

38-
public function checkDocument(Document $document): ?string {
65+
#[Override]
66+
public function prepareSession(DocumentData $documentData): SessionInfo|string {
67+
$document = $documentData->document;
68+
$documentState = $documentData->documentState;
69+
3970
if ($this->baseVersionEtag !== null && $this->baseVersionEtag !== $document->getBaseVersionEtag()) {
4071
return $this->l10n->t('Editing session has expired. Please reload the page.');
4172
}
42-
return null;
43-
}
4473

45-
public function isReadOnly(): bool {
46-
return $this->fileService->isReadOnly($this->file, $this->token);
47-
}
74+
$content = null;
75+
if ($documentState === null) {
76+
$this->logger->debug('Sending content for ' . $document->toString());
77+
$content = $this->loadContent();
78+
}
4879

49-
public function getId(): int {
50-
return $this->file->getId();
80+
$readOnly = $this->isReadOnly();
81+
$lockInfo = $this->getLockInfo();
82+
if (!$readOnly) {
83+
$isLocked = $this->lock();
84+
if (!$isLocked) {
85+
$readOnly = true;
86+
}
87+
}
88+
89+
return new SessionInfo(
90+
content: $content,
91+
readOnly: $readOnly,
92+
lock: $lockInfo,
93+
hasOwner: $this->getOwner() !== null,
94+
);
5195
}
5296

53-
public function getType(): string {
54-
return 'file';
97+
private function computeCheckSum(): string {
98+
return hash('crc32', $this->file->getContent());
5599
}
56100

57-
public function toString(): string {
58-
return $this->getType() . ' (' . $this->getId() . ')';
101+
private function isReadOnly(): bool {
102+
return $this->fileService->isReadOnly($this->file, $this->token);
59103
}
60104

61-
public function loadContent(): ?string {
105+
private function loadContent(): ?string {
62106
return $this->fileService->loadContent($this->file);
63107
}
64108

65-
public function getLockInfo(): ?ILock {
109+
private function getLockInfo(): ?ILock {
66110
return $this->lockService->getLockByOthers($this->file);
67111
}
68112

69-
public function getOwner(): ?IUser {
113+
private function getOwner(): ?IUser {
70114
return $this->file->getOwner();
71115
}
72116

73-
public function lock(): bool {
117+
private function lock(): bool {
74118
// Disable file locking for Readme.md files, because in the
75119
// current setup, this makes it almost impossible to delete these files.
76120
if (strcasecmp($this->file->getName(), 'Readme.md') !== 0) {
@@ -79,20 +123,4 @@ public function lock(): bool {
79123
return true;
80124
}
81125

82-
public function createDocument(): Document {
83-
$document = new Document();
84-
$document->setId($this->getId());
85-
$document->setLastSavedVersion(0);
86-
$document->setLastSavedVersionTime($this->file->getMTime());
87-
$document->setLastSavedVersionEtag($this->file->getEtag());
88-
$document->setChecksum($this->computeChecksum());
89-
// This is a new document - so it needs a fresh base version etag.
90-
$document->setBaseVersionEtag(uniqid());
91-
return $document;
92-
}
93-
94-
public function computeCheckSum(): string {
95-
return hash('crc32', $this->file->getContent());
96-
}
97-
98126
}

‎lib/Context/FileContextFactory.php‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,40 @@
1111
use OCA\Text\Service\LockService;
1212
use OCP\Constants;
1313
use OCP\DirectEditing\IToken;
14+
use OCP\Files\File;
1415
use OCP\Files\NotFoundException;
1516
use OCP\Files\NotPermittedException;
1617
use OCP\IL10N;
1718
use OCP\IUserSession;
19+
use Psr\Log\LoggerInterface;
1820

1921
class FileContextFactory {
2022

2123
public function __construct(
2224
private readonly FileService $fileService,
2325
private readonly IL10N $l10n,
2426
private readonly LockService $lockService,
27+
private readonly LoggerInterface $logger,
2528
private readonly IUserSession $userSession,
2629
) {
2730
}
2831

32+
private function build(
33+
File $file,
34+
?string $baseVersionEtag,
35+
?string $token = null,
36+
): FileContext {
37+
return new FileContext(
38+
$this->fileService,
39+
$this->l10n,
40+
$this->lockService,
41+
$this->logger,
42+
$file,
43+
$baseVersionEtag,
44+
$token,
45+
);
46+
}
47+
2948
/**
3049
* @throws NotPermittedException if not logged in
3150
* @throws NotFoundException if the file cannot be found
@@ -39,13 +58,7 @@ public function buildForId(
3958
throw new NotPermittedException();
4059
}
4160
$file = $this->fileService->getFileById($id, $userId);
42-
return new FileContext(
43-
$this->fileService,
44-
$this->l10n,
45-
$this->lockService,
46-
$file,
47-
$baseVersionEtag,
48-
);
61+
return $this->build($file, $baseVersionEtag);
4962
}
5063

5164
/**
@@ -59,28 +72,15 @@ public function buildForShareWithPath(
5972
): FileContext {
6073
$file = $this->fileService->getFileByShareToken($token, $filePath);
6174
$this->fileService->checkSharePermissions($token, Constants::PERMISSION_READ);
62-
return new FileContext(
63-
$this->fileService,
64-
$this->l10n,
65-
$this->lockService,
66-
$file,
67-
$baseVersionEtag,
68-
$token,
69-
);
75+
return $this->build($file, $baseVersionEtag, $token);
7076
}
7177

7278
/**
7379
* @throws NotFoundException if the file cannot be found
7480
*/
7581
public function buildForDirectEditing(IToken $token): FileContext {
7682
$file = $token->getFile();
77-
return new FileContext(
78-
$this->fileService,
79-
$this->l10n,
80-
$this->lockService,
81-
$file,
82-
null,
83-
);
83+
return $this->build($file, null);
8484
}
8585

8686
}

‎lib/Context/IContext.php‎

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,71 @@
88
namespace OCA\Text\Context;
99

1010
use OCA\Text\Db\Document;
11+
use OCA\Text\Db\Session;
1112
use OCP\Files\Lock\ILock;
1213
use OCP\IUser;
1314

1415
interface IContext {
15-
public function check(): ?string;
16-
public function checkDocument(Document $document): ?string;
17-
public function isReadOnly(): bool;
1816
public function getId(): int;
1917
public function getType(): string;
2018
public function toString(): string;
21-
public function loadContent(): ?string;
22-
public function getLockInfo(): ?ILock;
23-
public function getOwner(): ?IUser;
24-
public function lock(): bool;
25-
public function createDocument(): Document;
19+
public function buildDocument(): Document|string;
20+
public function prepareSession(DocumentData $documentData): SessionInfo|string;
21+
}
22+
23+
readonly class DocumentData {
24+
public function __construct(
25+
public Document $document,
26+
public ?string $documentState,
27+
) {
28+
}
29+
30+
public function jsonSerialize(): array {
31+
return [
32+
'document' => $this->document,
33+
'documentState' => $this->documentState,
34+
];
35+
}
36+
}
37+
38+
readonly class SessionInfo {
39+
public function __construct(
40+
public ?string $content,
41+
public bool $readOnly,
42+
public ?ILock $lock,
43+
public bool $hasOwner,
44+
) {
45+
}
46+
47+
public function jsonSerialize(): array {
48+
return [
49+
'content' => $this->content,
50+
'readOnly' => $this->readOnly,
51+
'lock' => $this->lock,
52+
'hasOwner' => $this->hasOwner,
53+
];
54+
}
55+
}
56+
57+
readonly class NewSessionData {
58+
public function __construct(
59+
public DocumentData $documentData,
60+
public SessionInfo $sessionInfo,
61+
public Session $session,
62+
public ?string $displayName,
63+
) {
64+
}
65+
66+
public function jsonSerialize(): array {
67+
return array_merge(
68+
$this->documentData->jsonSerialize(),
69+
$this->sessionInfo->jsonSerialize(),
70+
[
71+
'session' => array_merge(
72+
$this->session->jsonSerialize(),
73+
['displayName' => $this->displayName],
74+
),
75+
],
76+
);
77+
}
2678
}

‎lib/Db/Document.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,11 @@ public function jsonSerialize(): array {
5555
'checksum' => $this->checksum
5656
];
5757
}
58+
59+
/**
60+
* Short identifier - mostly for logging
61+
*/
62+
public function toString(): string {
63+
return 'file' . ' (' . $this->id . ')';
64+
}
5865
}

‎lib/Db/DocumentMapper.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace OCA\Text\Db;
99

1010
use Generator;
11+
use OCA\Text\Context\IContext;
1112
use OCP\AppFramework\Db\DoesNotExistException;
1213
use OCP\AppFramework\Db\QBMapper;
1314
use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -40,6 +41,14 @@ public function find(int $documentId): Document {
4041
return Document::fromRow($data);
4142
}
4243

44+
/**
45+
* @throws DoesNotExistException
46+
*/
47+
public function load(IContext $context): Document {
48+
$id = $context->getId();
49+
return $this->find($id);
50+
}
51+
4352
public function findAll(): Generator {
4453
$qb = $this->db->getQueryBuilder();
4554
$result = $qb->select('*')

0 commit comments

Comments
 (0)