diff --git a/lib/Controller/AttachmentController.php b/lib/Controller/AttachmentController.php index 28898655d88..68d2f50a38f 100644 --- a/lib/Controller/AttachmentController.php +++ b/lib/Controller/AttachmentController.php @@ -161,13 +161,17 @@ private function getUploadedFile(string $key): array { /** * Serve the image files in the editor + * + * @return DataDownloadResponse|DataResponse + * + * @psalm-return DataDownloadResponse<200, string, array>|DataResponse<404, '', array> */ #[NoAdminRequired] #[PublicPage] #[NoCSRFRequired] #[RequireDocumentSession] public function getImageFile(string $imageFileName, ?string $shareToken = null, - int $preferRawImage = 0) { + int $preferRawImage = 0): DataResponse|DataDownloadResponse { $documentId = $this->getSession()->getDocumentId(); try { @@ -192,12 +196,16 @@ public function getImageFile(string $imageFileName, ?string $shareToken = null, /** * Serve the media files in the editor + * + * @return DataDownloadResponse|DataResponse + * + * @psalm-return DataDownloadResponse<200, string, array>|DataResponse<404, '', array> */ #[NoAdminRequired] #[PublicPage] #[NoCSRFRequired] #[RequireDocumentSession] - public function getMediaFile(string $mediaFileName, ?string $shareToken = null) { + public function getMediaFile(string $mediaFileName, ?string $shareToken = null): DataResponse|DataDownloadResponse { $documentId = $this->getSession()->getDocumentId(); try { diff --git a/lib/Controller/PublicSessionController.php b/lib/Controller/PublicSessionController.php index a18735e1cce..a1cc09a07cd 100644 --- a/lib/Controller/PublicSessionController.php +++ b/lib/Controller/PublicSessionController.php @@ -40,7 +40,7 @@ class PublicSessionController extends PublicShareController implements ISessionAwareController { use TSessionAwareController; - private IShare $share; + private ?IShare $share = null; public function __construct( string $appName, @@ -52,8 +52,16 @@ public function __construct( parent::__construct($appName, $request, $session); } + private function getShare(): IShare { + if ($this->share === null) { + throw new \Exception('Share has not been set yet'); + } + + return $this->share; + } + protected function getPasswordHash(): string { - return $this->share->getPassword(); + return $this->getShare()->getPassword(); } public function isValidToken(): bool { @@ -66,12 +74,13 @@ public function isValidToken(): bool { } protected function isPasswordProtected(): bool { - return $this->share->getPassword() !== null; + /** @psalm-suppress RedundantConditionGivenDocblockType */ + return $this->getShare()->getPassword() !== null; } #[NoAdminRequired] #[PublicPage] - public function create(string $token, string $file = null, $guestName = null): DataResponse { + public function create(string $token, string $file = null, ?string $guestName = null): DataResponse { return $this->apiService->create(null, $file, $token, $guestName); } @@ -95,10 +104,13 @@ public function sync(string $token, int $documentId, int $sessionId, string $ses return $this->apiService->sync($this->getSession(), $this->getDocument(), $version, $autosaveContent, $documentState, $force, $manualSave, $token); } + /** + * @psalm-return DataResponse> + */ #[NoAdminRequired] #[PublicPage] #[RequireDocumentSession] - public function updateSession(string $guestName) { + public function updateSession(string $guestName): DataResponse { return $this->apiService->updateSession($this->getSession(), $guestName); } } diff --git a/lib/Controller/SessionController.php b/lib/Controller/SessionController.php index 66622e4a685..88fcd21754a 100644 --- a/lib/Controller/SessionController.php +++ b/lib/Controller/SessionController.php @@ -103,7 +103,7 @@ public function mention(string $mention): DataResponse { return new DataResponse($this->notificationService->mention($this->getDocument()->getId(), $mention)); } - private function loginSessionUser() { + private function loginSessionUser(): void { $currentSession = $this->getSession(); if (!$this->userSession->isLoggedIn()) { $user = $this->userManager->get($currentSession->getUserId()); diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index a901bc611e9..b1214cb5d59 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -32,25 +32,21 @@ use OCP\IRequest; class SettingsController extends Controller { - private IConfig $config; - private ?string $userId; - public const ACCEPTED_KEYS = [ 'workspace_enabled' ]; - public function __construct($appName, IRequest $request, IConfig $config, $userId) { + public function __construct(string $appName, IRequest $request, private IConfig $config, private ?string $userId) { parent::__construct($appName, $request); - - $this->config = $config; - $this->userId = $userId; } /** * @throws \OCP\PreConditionNotMetException + * + * @psalm-return DataResponse<200|400, array{workspace_enabled?: mixed, message?: 'Invalid config key'}, array> */ #[NoAdminRequired] - public function updateConfig(string $key, $value) { + public function updateConfig(string $key, string $value): DataResponse { if (!in_array($key, self::ACCEPTED_KEYS, true)) { return new DataResponse(['message' => 'Invalid config key'], Http::STATUS_BAD_REQUEST); } diff --git a/lib/Controller/WorkspaceController.php b/lib/Controller/WorkspaceController.php index dd76762e639..b55d8613597 100644 --- a/lib/Controller/WorkspaceController.php +++ b/lib/Controller/WorkspaceController.php @@ -145,6 +145,7 @@ public function publicFolder(string $shareToken, string $path = '/'): DataRespon if (!($share->getPermissions() & Constants::PERMISSION_READ)) { throw new ShareNotFound(); } + /** @psalm-suppress RedundantConditionGivenDocblockType */ if ($share->getPassword() !== null) { $shareId = $this->session->get('public_link_authenticated'); if ($share->getId() !== $shareId) { diff --git a/lib/Cron/Cleanup.php b/lib/Cron/Cleanup.php index 0ee0776f5b9..19933cfc17c 100644 --- a/lib/Cron/Cleanup.php +++ b/lib/Cron/Cleanup.php @@ -54,6 +54,10 @@ public function __construct(ITimeFactory $time, $this->setInterval(SessionService::SESSION_VALID_TIME); } + /** + * @param array $argument + * @return void + */ protected function run($argument) { $this->logger->debug('Run cleanup job for text documents'); $documents = $this->documentService->getAll(); diff --git a/lib/Db/Document.php b/lib/Db/Document.php index bf3d0430ec5..69f89fabe59 100644 --- a/lib/Db/Document.php +++ b/lib/Db/Document.php @@ -41,7 +41,7 @@ * @method setBaseVersionEtag(string $etag): void */ class Document extends Entity implements \JsonSerializable { - public $id; + public $id = null; // TODO: Remove obsolete field `currentVersion` protected int $currentVersion = 0; protected int $lastSavedVersion = 0; diff --git a/lib/Db/DocumentMapper.php b/lib/Db/DocumentMapper.php index ec30cf37016..13b878b7de2 100644 --- a/lib/Db/DocumentMapper.php +++ b/lib/Db/DocumentMapper.php @@ -39,7 +39,7 @@ public function __construct(IDBConnection $db) { * @return Document * @throws DoesNotExistException */ - public function find($documentId): Document { + public function find(int $documentId): Document { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $result = $qb->select('*') diff --git a/lib/Db/SessionMapper.php b/lib/Db/SessionMapper.php index 03ca252f65a..8465d1bf5d6 100644 --- a/lib/Db/SessionMapper.php +++ b/lib/Db/SessionMapper.php @@ -53,7 +53,7 @@ public function findAllDocuments(): array { * @return Session * @throws DoesNotExistException */ - public function find($documentId, $sessionId, $token): Session { + public function find(int $documentId, int $sessionId, string $token): Session { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $result = $qb->select('*') @@ -71,7 +71,12 @@ public function find($documentId, $sessionId, $token): Session { return Session::fromRow($data); } - public function findAll($documentId) { + /** + * @return Session[] + * + * @psalm-return array + */ + public function findAll(int $documentId): array { $qb = $this->db->getQueryBuilder(); $qb->select('id', 'color', 'document_id', 'last_awareness_message', 'last_contact', 'user_id', 'guest_name') ->from($this->getTableName()) @@ -80,7 +85,12 @@ public function findAll($documentId) { return $this->findEntities($qb); } - public function findAllActive($documentId) { + /** + * @return Session[] + * + * @psalm-return array + */ + public function findAllActive(int $documentId): array { $qb = $this->db->getQueryBuilder(); $qb->select('id', 'color', 'document_id', 'last_awareness_message', 'last_contact', 'user_id', 'guest_name') ->from($this->getTableName()) @@ -90,7 +100,12 @@ public function findAllActive($documentId) { return $this->findEntities($qb); } - public function findAllInactive() { + /** + * @return Session[] + * + * @psalm-return array + */ + public function findAllInactive(): array { $qb = $this->db->getQueryBuilder(); $qb->select('id', 'color', 'document_id', 'last_awareness_message', 'last_contact', 'user_id', 'guest_name') ->from($this->getTableName()) @@ -132,14 +147,14 @@ public function deleteInactiveWithoutSteps(?int $documentId = null): int { return $deletedCount; } - public function deleteByDocumentId($documentId): int { + public function deleteByDocumentId(int $documentId): int { $qb = $this->db->getQueryBuilder(); $qb->delete($this->getTableName()) ->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId))); return $qb->executeStatement(); } - public function isUserInDocument($documentId, $userId): bool { + public function isUserInDocument(int $documentId, string $userId): bool { $qb = $this->db->getQueryBuilder(); $result = $qb->select('*') ->from($this->getTableName()) diff --git a/lib/Db/Step.php b/lib/Db/Step.php index 92ae39d2d08..b2c1ff1aa63 100644 --- a/lib/Db/Step.php +++ b/lib/Db/Step.php @@ -37,6 +37,7 @@ * @method setDocumentId(int $documentId): void */ class Step extends Entity implements JsonSerializable { + public $id = null; protected string $data = ''; protected int $version = 0; protected int $sessionId = 0; @@ -55,10 +56,10 @@ public function jsonSerialize(): array { throw new \InvalidArgumentException('Failed to parse step data'); } return [ - 'id' => $this->id, + 'id' => $this->getId(), 'data' => $jsonData, - 'version' => $this->version, - 'sessionId' => $this->sessionId + 'version' => $this->getVersion(), + 'sessionId' => $this->getSessionId() ]; } } diff --git a/lib/Db/StepMapper.php b/lib/Db/StepMapper.php index ae4c7d9c88d..11b7d59f894 100644 --- a/lib/Db/StepMapper.php +++ b/lib/Db/StepMapper.php @@ -33,7 +33,10 @@ public function __construct(IDBConnection $db) { parent::__construct($db, 'text_steps', Step::class); } - public function find($documentId, $fromVersion, $lastAckedVersion = null) { + /** + * @return Step[] + */ + public function find(int $documentId, int $fromVersion, int $lastAckedVersion = null): array { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->select('*') @@ -51,7 +54,7 @@ public function find($documentId, $fromVersion, $lastAckedVersion = null) { return $this->findEntities($qb); } - public function getLatestVersion($documentId): ?int { + public function getLatestVersion(int $documentId): ?int { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $result = $qb->select('version') @@ -69,22 +72,22 @@ public function getLatestVersion($documentId): ?int { return $data['version']; } - public function deleteAll($documentId): void { + public function deleteAll(int $documentId): void { $qb = $this->db->getQueryBuilder(); $qb->delete($this->getTableName()) ->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId))) ->executeStatement(); } - public function deleteBeforeVersion($documentId, $version): void { + public function deleteBeforeVersion(int $documentId, int $version): int { $qb = $this->db->getQueryBuilder(); - $qb->delete($this->getTableName()) + return $qb->delete($this->getTableName()) ->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId))) ->andWhere($qb->expr()->lte('version', $qb->createNamedParameter($version))) ->executeStatement(); } - public function deleteAfterVersion($documentId, $version): int { + public function deleteAfterVersion(int $documentId, int $version): int { $qb = $this->db->getQueryBuilder(); return $qb->delete($this->getTableName()) ->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId))) diff --git a/lib/Migration/Version010000Date20190617184535.php b/lib/Migration/Version010000Date20190617184535.php index eb1e5dd1705..ee52e9d02fe 100644 --- a/lib/Migration/Version010000Date20190617184535.php +++ b/lib/Migration/Version010000Date20190617184535.php @@ -19,6 +19,8 @@ class Version010000Date20190617184535 extends SimpleMigrationStep { * @param IOutput $output * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` * @param array $options + * + * @return void */ public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { } @@ -139,6 +141,8 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt * @param IOutput $output * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` * @param array $options + * + * @return void */ public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { } diff --git a/lib/Service/ApiService.php b/lib/Service/ApiService.php index c7cecb5420e..609f848b7cf 100644 --- a/lib/Service/ApiService.php +++ b/lib/Service/ApiService.php @@ -69,7 +69,7 @@ public function __construct(IRequest $request, $this->l10n = $l10n; } - public function create($fileId = null, $filePath = null, ?string $token = null, $guestName = null): DataResponse { + public function create(?int $fileId = null, ?string $filePath = null, ?string $token = null, ?string $guestName = null): DataResponse { try { if ($token) { $file = $this->documentService->getFileByShareToken($token, $this->request->getParam('filePath')); @@ -170,7 +170,7 @@ public function create($fileId = null, $filePath = null, ?string $token = null, ]); } - public function close($documentId, $sessionId, $sessionToken): DataResponse { + public function close(int $documentId, int $sessionId, string $sessionToken): DataResponse { $this->sessionService->closeSession($documentId, $sessionId, $sessionToken); $this->sessionService->removeInactiveSessionsWithoutSteps($documentId); $activeSessions = $this->sessionService->getActiveSessions($documentId); @@ -183,8 +183,10 @@ public function close($documentId, $sessionId, $sessionToken): DataResponse { /** * @throws NotFoundException * @throws DoesNotExistException + * + * @param null|string $token */ - public function push(Session $session, Document $document, $version, $steps, $awareness, $token = null): DataResponse { + public function push(Session $session, Document $document, int $version, array $steps, string $awareness, string|null $token = null): DataResponse { try { $session = $this->sessionService->updateSessionAwareness($session, $awareness); } catch (DoesNotExistException $e) { @@ -209,7 +211,12 @@ public function push(Session $session, Document $document, $version, $steps, $aw return new DataResponse($result); } - public function sync(Session $session, Document $document, $version = 0, $autosaveContent = null, $documentState = null, bool $force = false, bool $manualSave = false, $token = null): DataResponse { + /** + * @param null|string $autosaveContent + * @param null|string $documentState + * @param null|string $token + */ + public function sync(Session $session, Document $document, int $version = 0, string|null $autosaveContent = null, string|null $documentState = null, bool $force = false, bool $manualSave = false, string|null $token = null): DataResponse { $documentId = $session->getDocumentId(); try { $result = [ diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php index eccae278aea..459e9981cfe 100644 --- a/lib/Service/AttachmentService.php +++ b/lib/Service/AttachmentService.php @@ -272,7 +272,10 @@ private function getMediaFileMetadata(string $mediaFileName, File $textFile): ?a * @param string $newFileName * @param string $newFileContent * @param string $userId + * @param resource $newFileResource + * * @return array + * * @throws NotFoundException * @throws NotPermittedException * @throws \OCP\Files\InvalidPathException @@ -301,7 +304,10 @@ public function uploadAttachment(int $documentId, string $newFileName, $newFileR * @param string $newFileName * @param string $newFileContent * @param string $shareToken + * @param resource $newFileResource + * * @return array + * * @throws NotFoundException * @throws NotPermittedException * @throws \OCP\Files\InvalidPathException diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index ae475628ece..ce6de82c132 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -86,7 +86,7 @@ class DocumentService { private ILockManager $lockManager; private IUserMountCache $userMountCache; - public function __construct(DocumentMapper $documentMapper, StepMapper $stepMapper, SessionMapper $sessionMapper, IAppData $appData, $userId, IRootFolder $rootFolder, ICacheFactory $cacheFactory, LoggerInterface $logger, ShareManager $shareManager, IRequest $request, IManager $directManager, ILockingProvider $lockingProvider, ILockManager $lockManager, IUserMountCache $userMountCache) { + public function __construct(DocumentMapper $documentMapper, StepMapper $stepMapper, SessionMapper $sessionMapper, IAppData $appData, ?string $userId, IRootFolder $rootFolder, ICacheFactory $cacheFactory, LoggerInterface $logger, ShareManager $shareManager, IRequest $request, IManager $directManager, ILockingProvider $lockingProvider, ILockManager $lockManager, IUserMountCache $userMountCache) { $this->documentMapper = $documentMapper; $this->stepMapper = $stepMapper; $this->sessionMapper = $sessionMapper; @@ -206,15 +206,10 @@ public function writeDocumentState(int $documentId, string $content): void { } /** - * @param $documentId - * @param $sessionId - * @param $steps - * @param $version - * @return array * @throws DoesNotExistException * @throws InvalidArgumentException */ - public function addStep(Document $document, Session $session, $steps, $version): array { + public function addStep(Document $document, Session $session, array $steps, int $version): array { $sessionId = $session->getId(); $documentId = $session->getDocumentId(); $stepsToInsert = []; @@ -255,17 +250,18 @@ public function addStep(Document $document, Session $session, $steps, $version): * @param $sessionId * @param $steps * @param $version + * * @return int + * * @throws DoesNotExistException * @throws InvalidArgumentException + * + * @psalm-param non-empty-list $steps */ - private function insertSteps(Document $document, Session $session, $steps, $version): int { + private function insertSteps(Document $document, Session $session, array $steps, ?int $version): int { $stepsVersion = null; try { - $stepsJson = json_encode($steps); - if (!is_array($steps) || $stepsJson === null) { - throw new InvalidArgumentException('Failed to encode steps'); - } + $stepsJson = json_encode($steps, JSON_THROW_ON_ERROR); $stepsVersion = $this->stepMapper->getLatestVersion($document->getId()); $newVersion = $stepsVersion + count($steps); $this->logger->debug("Adding steps to " . $document->getId() . ": bumping version from $stepsVersion to $newVersion"); @@ -288,7 +284,7 @@ private function insertSteps(Document $document, Session $session, $steps, $vers } } - public function getSteps($documentId, $lastVersion): array { + public function getSteps(int $documentId, int $lastVersion): array { if ($lastVersion === $this->cache->get('document-version-' . $documentId)) { return []; } @@ -422,7 +418,7 @@ public function resetDocument(int $documentId, bool $force = false): void { } } - public function getAll() { + public function getAll(): array { return $this->documentMapper->findAll(); } @@ -519,7 +515,7 @@ public function getFileByShareToken(string $shareToken, ?string $path = null): F } - public function isReadOnly($file, $token) { + public function isReadOnly(File $file, string|null $token): bool { $readOnly = true; if ($token) { try { @@ -543,7 +539,7 @@ public function isReadOnly($file, $token) { return $readOnly || $lockInfo !== null; } - public function getLockInfo($file): ?ILock { + public function getLockInfo(File $file): ?ILock { try { $locks = $this->lockManager->getLocks($file->getId()); } catch (NoLockProviderException|PreConditionNotMetException $e) { @@ -554,10 +550,14 @@ public function getLockInfo($file): ?ILock { /** * @param $shareToken + * * @return void + * * @throws NotFoundException|NotPermittedException + * + * @psalm-param 1|2 $permission */ - public function checkSharePermissions(string $shareToken, $permission = Constants::PERMISSION_READ): void { + public function checkSharePermissions(string $shareToken, int $permission = Constants::PERMISSION_READ): void { try { $share = $this->shareManager->getShareByToken($shareToken); } catch (ShareNotFound $e) { @@ -569,7 +569,7 @@ public function checkSharePermissions(string $shareToken, $permission = Constant } } - public function hasUnsavedChanges(Document $document) { + public function hasUnsavedChanges(Document $document): bool { $stepsVersion = $this->stepMapper->getLatestVersion($document->getId()) ?: 0; $docVersion = $document->getLastSavedVersion(); return $stepsVersion !== $docVersion; diff --git a/lib/Service/SessionService.php b/lib/Service/SessionService.php index 480677b7357..569991b2c7e 100644 --- a/lib/Service/SessionService.php +++ b/lib/Service/SessionService.php @@ -60,7 +60,7 @@ public function __construct( IAvatarManager $avatarManager, IRequest $request, IManager $directManager, - $userId, + ?string $userId, ICacheFactory $cacheFactory ) { $this->sessionMapper = $sessionMapper; @@ -84,7 +84,7 @@ public function __construct( $this->cache = $cacheFactory->createDistributed('text_sessions'); } - public function initSession($documentId, $guestName = null): Session { + public function initSession(int $documentId, string $guestName = null): Session { $session = new Session(); $session->setDocumentId($documentId); $userName = $this->userId ? $this->userId : $guestName; @@ -92,7 +92,7 @@ public function initSession($documentId, $guestName = null): Session { $session->setToken($this->secureRandom->generate(64)); $session->setColor($this->getColorForGuestName($guestName)); if ($this->userId === null) { - $session->setGuestName($guestName); + $session->setGuestName($guestName ?? ''); } $session->setLastContact($this->timeFactory->now()->getTimestamp()); @@ -141,11 +141,12 @@ public function getNameForSession(Session $session): ?string { return $session->getGuestName(); } - public function findAllInactive() { + /** @return Session[] */ + public function findAllInactive(): array { return $this->sessionMapper->findAllInactive(); } - public function removeInactiveSessionsWithoutSteps(?int $documentId = null) { + public function removeInactiveSessionsWithoutSteps(?int $documentId = null): int { // No need to clear the cache here as we already set a TTL return $this->sessionMapper->deleteInactiveWithoutSteps($documentId); } @@ -205,16 +206,7 @@ public function getValidSession(int $documentId, int $sessionId, string $token): return $session; } - public function isValidSession($documentId, $sessionId, $token): bool { - return $this->getValidSession($documentId, $sessionId, $token) !== null; - } - /** - * @param $documentId - * @param $sessionId - * @param $sessionToken - * @param $guestName - * @return Session * @throws DoesNotExistException */ public function updateSession(Session $session, string $guestName): Session { @@ -227,11 +219,6 @@ public function updateSession(Session $session, string $guestName): Session { } /** - * @param $documentId - * @param $sessionId - * @param $sessionToken - * @param $message - * @return Session * @throws DoesNotExistException */ public function updateSessionAwareness(Session $session, string $message): Session { diff --git a/psalm.xml b/psalm.xml index ff84f35c427..62b01cf6352 100644 --- a/psalm.xml +++ b/psalm.xml @@ -1,6 +1,6 @@