From ee7df4593246a29d9748cfa4a47b7b5a5ea0d3f7 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 20 Aug 2026 18:32:14 +0200 Subject: [PATCH 1/3] refactor: Port OpenLocalEditor to ORM system Signed-off-by: Carl Schwan --- .../Controller/DirectEditingController.php | 12 +-- .../DirectEditingViewController.php | 5 +- .../Controller/OpenLocalEditorController.php | 35 ++++--- apps/files/lib/Db/OpenLocalEditor.php | 43 ++++----- apps/files/lib/Db/OpenLocalEditorMapper.php | 34 +++---- .../tests/Db/OpenLocalEditorMapperTest.php | 95 +++++++++++++++++++ 6 files changed, 149 insertions(+), 75 deletions(-) create mode 100644 apps/files/tests/Db/OpenLocalEditorMapperTest.php diff --git a/apps/files/lib/Controller/DirectEditingController.php b/apps/files/lib/Controller/DirectEditingController.php index 6159de224079c..97876e6c9391a 100644 --- a/apps/files/lib/Controller/DirectEditingController.php +++ b/apps/files/lib/Controller/DirectEditingController.php @@ -52,9 +52,9 @@ public function info(): DataResponse { /** * Create a file for direct editing * - * @param string $path Path of the file - * @param string $editorId ID of the editor - * @param string $creatorId ID of the creator + * @param non-empty-string $path Path of the file + * @param non-empty-string $editorId ID of the editor + * @param non-empty-string $creatorId ID of the creator * @param ?string $templateId ID of the template * * @return DataResponse|DataResponse @@ -88,7 +88,7 @@ public function create(string $path, string $editorId, string $creatorId, ?strin /** * Open a file for direct editing * - * @param string $path Path of the file + * @param non-empty-string $path Path of the file * @param ?string $editorId ID of the editor * @param ?int $fileId ID of the file * @@ -123,8 +123,8 @@ public function open(string $path, ?string $editorId = null, ?int $fileId = null /** * Get the templates for direct editing * - * @param string $editorId ID of the editor - * @param string $creatorId ID of the creator + * @param non-empty-string $editorId ID of the editor + * @param non-empty-string $creatorId ID of the creator * * @return DataResponse}, array{}>|DataResponse * diff --git a/apps/files/lib/Controller/DirectEditingViewController.php b/apps/files/lib/Controller/DirectEditingViewController.php index 76f2acd112e08..35099bf82ad2b 100644 --- a/apps/files/lib/Controller/DirectEditingViewController.php +++ b/apps/files/lib/Controller/DirectEditingViewController.php @@ -24,7 +24,7 @@ #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] class DirectEditingViewController extends Controller { public function __construct( - $appName, + string $appName, IRequest $request, private IEventDispatcher $eventDispatcher, private IManager $directEditingManager, @@ -34,8 +34,7 @@ public function __construct( } /** - * @param string $token - * @return Response + * @param non-empty-string $token */ #[PublicPage] #[NoCSRFRequired] diff --git a/apps/files/lib/Controller/OpenLocalEditorController.php b/apps/files/lib/Controller/OpenLocalEditorController.php index b000304eef668..399ab7f7efb73 100644 --- a/apps/files/lib/Controller/OpenLocalEditorController.php +++ b/apps/files/lib/Controller/OpenLocalEditorController.php @@ -21,6 +21,7 @@ use OCP\AppFramework\Utility\ITimeFactory; use OCP\DB\Exception; use OCP\IRequest; +use OCP\IUser; use OCP\Security\ISecureRandom; use Psr\Log\LoggerInterface; @@ -36,7 +37,6 @@ public function __construct( protected OpenLocalEditorMapper $mapper, protected ISecureRandom $secureRandom, protected LoggerInterface $logger, - protected ?string $userId, ) { parent::__construct($appName, $request); } @@ -52,26 +52,26 @@ public function __construct( */ #[NoAdminRequired] #[UserRateLimit(limit: 10, period: 120)] - public function create(string $path): DataResponse { + public function create(IUser $user, string $path): DataResponse { $pathHash = sha1($path); $entity = new OpenLocalEditor(); - $entity->setUserId($this->userId); - $entity->setPathHash($pathHash); - $entity->setExpirationTime($this->timeFactory->getTime() + self::TOKEN_DURATION); // Expire in 10 minutes + $entity->userId = $user->getUID(); + $entity->pathHash = $pathHash; + $entity->expirationTime = $this->timeFactory->getTime() + self::TOKEN_DURATION; // Expire in 10 minutes for ($i = 1; $i <= self::TOKEN_RETRIES; $i++) { $token = $this->secureRandom->generate(self::TOKEN_LENGTH, ISecureRandom::CHAR_ALPHANUMERIC); - $entity->setToken($token); + $entity->token = $token; try { $this->mapper->insert($entity); return new DataResponse([ - 'userId' => $this->userId, + 'userId' => $user->getUID(), 'pathHash' => $pathHash, - 'expirationTime' => $entity->getExpirationTime(), - 'token' => $entity->getToken(), + 'expirationTime' => $entity->expirationTime, + 'token' => $entity->token, ]); } catch (Exception $e) { if ($e->getCode() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { @@ -98,31 +98,30 @@ public function create(string $path): DataResponse { */ #[NoAdminRequired] #[BruteForceProtection(action: 'openLocalEditor')] - public function validate(string $path, string $token): DataResponse { + public function validate(IUser $user, string $path, string $token): DataResponse { $pathHash = sha1($path); try { - $entity = $this->mapper->verifyToken($this->userId, $pathHash, $token); + $entity = $this->mapper->verifyToken($user->getUID(), $pathHash, $token); } catch (DoesNotExistException $e) { $response = new DataResponse([], Http::STATUS_NOT_FOUND); - $response->throttle(['userId' => $this->userId, 'pathHash' => $pathHash]); + $response->throttle(['userId' => $user->getUID(), 'pathHash' => $pathHash]); return $response; } $this->mapper->delete($entity); - if ($entity->getExpirationTime() <= $this->timeFactory->getTime()) { + if ($entity->expirationTime <= $this->timeFactory->getTime()) { $response = new DataResponse([], Http::STATUS_NOT_FOUND); - $response->throttle(['userId' => $this->userId, 'pathHash' => $pathHash]); + $response->throttle(['userId' => $user->getUID(), 'pathHash' => $pathHash]); return $response; } return new DataResponse([ - 'userId' => $this->userId, + 'userId' => $user->getUID(), 'pathHash' => $pathHash, - 'expirationTime' => $entity->getExpirationTime(), - 'token' => $entity->getToken(), + 'expirationTime' => $entity->expirationTime, + 'token' => $entity->token, ]); } - } diff --git a/apps/files/lib/Db/OpenLocalEditor.php b/apps/files/lib/Db/OpenLocalEditor.php index da7f5d1320637..be5e434b03c90 100644 --- a/apps/files/lib/Db/OpenLocalEditor.php +++ b/apps/files/lib/Db/OpenLocalEditor.php @@ -9,35 +9,26 @@ namespace OCA\Files\Db; -use OCP\AppFramework\Db\Entity; +use OCP\AppFramework\ORM\Attribute\Column; +use OCP\AppFramework\ORM\Attribute\Entity; +use OCP\AppFramework\ORM\Attribute\Id; +use OCP\DB\Schema\ColumnType; -/** - * @method void setUserId(string $userId) - * @method string getUserId() - * @method void setPathHash(string $pathHash) - * @method string getPathHash() - * @method void setExpirationTime(int $expirationTime) - * @method int getExpirationTime() - * @method void setToken(string $token) - * @method string getToken() - */ -class OpenLocalEditor extends Entity { - /** @var string */ - protected $userId; +#[Entity(name: 'open_local_editor')] +final class OpenLocalEditor { + #[Id] + #[Column(name: 'id', type: ColumnType::Bigint)] + public int $id; - /** @var string */ - protected $pathHash; + #[Column(name: 'user_id', type: ColumnType::String, length: 64)] + public string $userId; - /** @var int */ - protected $expirationTime; + #[Column(name: 'path_hash', type: ColumnType::String, length: 64)] + public string $pathHash; - /** @var string */ - protected $token; + #[Column(name: 'expiration_time', type: ColumnType::Bigint)] + public int $expirationTime; - public function __construct() { - $this->addType('userId', 'string'); - $this->addType('pathHash', 'string'); - $this->addType('expirationTime', 'integer'); - $this->addType('token', 'string'); - } + #[Column(name: 'token', type: ColumnType::String, length: 128)] + public string $token; } diff --git a/apps/files/lib/Db/OpenLocalEditorMapper.php b/apps/files/lib/Db/OpenLocalEditorMapper.php index 6ae8b79c25886..01dc9f1afd1d5 100644 --- a/apps/files/lib/Db/OpenLocalEditorMapper.php +++ b/apps/files/lib/Db/OpenLocalEditorMapper.php @@ -10,41 +10,31 @@ namespace OCA\Files\Db; use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\MultipleObjectsReturnedException; -use OCP\AppFramework\Db\QBMapper; -use OCP\DB\Exception; -use OCP\IDBConnection; +use OCP\AppFramework\ORM\Repository; +use OCP\DB\QueryBuilder\IQueryBuilder; /** - * @template-extends QBMapper + * @template-extends Repository */ -class OpenLocalEditorMapper extends QBMapper { - public function __construct(IDBConnection $db) { - parent::__construct($db, 'open_local_editor', OpenLocalEditor::class); - } +class OpenLocalEditorMapper extends Repository { + public const string entityClass = OpenLocalEditor::class; /** * @throws DoesNotExistException - * @throws MultipleObjectsReturnedException - * @throws Exception */ public function verifyToken(string $userId, string $pathHash, string $token): OpenLocalEditor { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))) - ->andWhere($qb->expr()->eq('path_hash', $qb->createNamedParameter($pathHash))) - ->andWhere($qb->expr()->eq('token', $qb->createNamedParameter($token))); - - return $this->findEntity($qb); + return $this->findOneBy([ + 'userId' => $userId, + 'pathHash' => $pathHash, + 'token' => $token, + ]); } public function deleteExpiredTokens(int $time): void { - $qb = $this->db->getQueryBuilder(); + $qb = $this->connection->getQueryBuilder(); $qb->delete($this->getTableName()) - ->where($qb->expr()->lt('expiration_time', $qb->createNamedParameter($time))); + ->where($qb->expr()->lt('expiration_time', $qb->createNamedParameter($time, IQueryBuilder::PARAM_INT))); $qb->executeStatement(); } diff --git a/apps/files/tests/Db/OpenLocalEditorMapperTest.php b/apps/files/tests/Db/OpenLocalEditorMapperTest.php new file mode 100644 index 0000000000000..739308a4822dc --- /dev/null +++ b/apps/files/tests/Db/OpenLocalEditorMapperTest.php @@ -0,0 +1,95 @@ +db->getQueryBuilder(); + $qb->delete($this->mapper->getTableName()) + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($this->testUID))); + $qb->executeStatement(); + } + + protected function setUp(): void { + parent::setUp(); + + $this->db = Server::get(IDBConnection::class); + $this->mapper = Server::get(OpenLocalEditorMapper::class); + + $this->resetDB(); + } + + protected function tearDown(): void { + parent::tearDown(); + + $this->resetDB(); + } + + private function createEntry(string $pathHash, string $token, int $expirationTime): OpenLocalEditor { + $entity = new OpenLocalEditor(); + $entity->userId = $this->testUID; + $entity->pathHash = $pathHash; + $entity->token = $token; + $entity->expirationTime = $expirationTime; + + return $this->mapper->insert($entity); + } + + public function testVerifyToken(): void { + $inserted = $this->createEntry('pathHash', 'thetoken', 1000); + + $found = $this->mapper->verifyToken($this->testUID, 'pathHash', 'thetoken'); + + $this->assertSame($inserted->id, $found->id); + $this->assertSame($this->testUID, $found->userId); + $this->assertSame('pathHash', $found->pathHash); + $this->assertSame('thetoken', $found->token); + $this->assertSame(1000, $found->expirationTime); + } + + public function testVerifyTokenNotFound(): void { + $this->createEntry('pathHash', 'thetoken', 1000); + + $this->expectException(DoesNotExistException::class); + $this->mapper->verifyToken($this->testUID, 'pathHash', 'wrongtoken'); + } + + public function testDeleteExpiredTokens(): void { + $this->createEntry('expired', 'expiredtoken', 1000); + $this->createEntry('valid', 'validtoken', 3000); + + $this->mapper->deleteExpiredTokens(2000); + + $this->expectException(DoesNotExistException::class); + $this->mapper->verifyToken($this->testUID, 'expired', 'expiredtoken'); + } + + public function testDeleteExpiredTokensKeepsUnexpired(): void { + $this->createEntry('expired', 'expiredtoken', 1000); + $valid = $this->createEntry('valid', 'validtoken', 3000); + + $this->mapper->deleteExpiredTokens(2000); + + $found = $this->mapper->verifyToken($this->testUID, 'valid', 'validtoken'); + $this->assertSame($valid->id, $found->id); + } +} From 8285e1f80397d1c2dc564b542dadadac991db3db Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 20 Aug 2026 18:40:12 +0200 Subject: [PATCH 2/3] refactor: Port TransferOwnership to ORM Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: Carl Schwan --- .../lib/BackgroundJob/TransferOwnership.php | 46 ++++----- .../Controller/OpenLocalEditorController.php | 12 +-- .../TransferOwnershipController.php | 30 +++--- apps/files/lib/Db/TransferOwnership.php | 43 ++++----- apps/files/lib/Db/TransferOwnershipMapper.php | 21 +---- apps/files/lib/Notification/Notifier.php | 12 +-- .../tests/Db/TransferOwnershipMapperTest.php | 94 +++++++++++++++++++ 7 files changed, 166 insertions(+), 92 deletions(-) create mode 100644 apps/files/tests/Db/TransferOwnershipMapperTest.php diff --git a/apps/files/lib/BackgroundJob/TransferOwnership.php b/apps/files/lib/BackgroundJob/TransferOwnership.php index 9960d10d3a007..55623a97a25cf 100644 --- a/apps/files/lib/BackgroundJob/TransferOwnership.php +++ b/apps/files/lib/BackgroundJob/TransferOwnership.php @@ -41,9 +41,9 @@ protected function run($argument) { $id = $argument['id']; $transfer = $this->mapper->getById($id); - $sourceUser = $transfer->getSourceUser(); - $destinationUser = $transfer->getTargetUser(); - $fileId = $transfer->getFileId(); + $sourceUser = $transfer->sourceUser; + $destinationUser = $transfer->targetUser; + $fileId = $transfer->fileId; $userFolder = $this->rootFolder->getUserFolder($sourceUser); $node = $userFolder->getFirstNodeById($fileId); @@ -93,55 +93,55 @@ protected function run($argument) { private function failedNotication(Transfer $transfer): void { // Send notification to source user $notification = $this->notificationManager->createNotification(); - $notification->setUser($transfer->getSourceUser()) + $notification->setUser($transfer->sourceUser) ->setApp(Application::APP_ID) ->setDateTime($this->time->getDateTime()) ->setSubject('transferOwnershipFailedSource', [ - 'sourceUser' => $transfer->getSourceUser(), - 'targetUser' => $transfer->getTargetUser(), - 'nodeName' => $transfer->getNodeName(), + 'sourceUser' => $transfer->sourceUser, + 'targetUser' => $transfer->targetUser, + 'nodeName' => $transfer->nodeName, ]) - ->setObject('transfer', (string)$transfer->getId()); + ->setObject('transfer', (string)$transfer->id); $this->notificationManager->notify($notification); // Send notification to source user $notification = $this->notificationManager->createNotification(); - $notification->setUser($transfer->getTargetUser()) + $notification->setUser($transfer->targetUser) ->setApp(Application::APP_ID) ->setDateTime($this->time->getDateTime()) ->setSubject('transferOwnershipFailedTarget', [ - 'sourceUser' => $transfer->getSourceUser(), - 'targetUser' => $transfer->getTargetUser(), - 'nodeName' => $transfer->getNodeName(), + 'sourceUser' => $transfer->sourceUser, + 'targetUser' => $transfer->targetUser, + 'nodeName' => $transfer->nodeName, ]) - ->setObject('transfer', (string)$transfer->getId()); + ->setObject('transfer', (string)$transfer->id); $this->notificationManager->notify($notification); } private function successNotification(Transfer $transfer): void { // Send notification to source user $notification = $this->notificationManager->createNotification(); - $notification->setUser($transfer->getSourceUser()) + $notification->setUser($transfer->sourceUser) ->setApp(Application::APP_ID) ->setDateTime($this->time->getDateTime()) ->setSubject('transferOwnershipDoneSource', [ - 'sourceUser' => $transfer->getSourceUser(), - 'targetUser' => $transfer->getTargetUser(), - 'nodeName' => $transfer->getNodeName(), + 'sourceUser' => $transfer->sourceUser, + 'targetUser' => $transfer->targetUser, + 'nodeName' => $transfer->nodeName, ]) - ->setObject('transfer', (string)$transfer->getId()); + ->setObject('transfer', (string)$transfer->id); $this->notificationManager->notify($notification); // Send notification to source user $notification = $this->notificationManager->createNotification(); - $notification->setUser($transfer->getTargetUser()) + $notification->setUser($transfer->targetUser) ->setApp(Application::APP_ID) ->setDateTime($this->time->getDateTime()) ->setSubject('transferOwnershipDoneTarget', [ - 'sourceUser' => $transfer->getSourceUser(), - 'targetUser' => $transfer->getTargetUser(), - 'nodeName' => $transfer->getNodeName(), + 'sourceUser' => $transfer->sourceUser, + 'targetUser' => $transfer->targetUser, + 'nodeName' => $transfer->nodeName, ]) - ->setObject('transfer', (string)$transfer->getId()); + ->setObject('transfer', (string)$transfer->id); $this->notificationManager->notify($notification); } } diff --git a/apps/files/lib/Controller/OpenLocalEditorController.php b/apps/files/lib/Controller/OpenLocalEditorController.php index 399ab7f7efb73..470ff1d8f8a7d 100644 --- a/apps/files/lib/Controller/OpenLocalEditorController.php +++ b/apps/files/lib/Controller/OpenLocalEditorController.php @@ -26,9 +26,9 @@ use Psr\Log\LoggerInterface; class OpenLocalEditorController extends OCSController { - public const TOKEN_LENGTH = 128; - public const TOKEN_DURATION = 600; // 10 Minutes - public const TOKEN_RETRIES = 50; + public const int TOKEN_LENGTH = 128; + public const int TOKEN_DURATION = 600; // 10 Minutes + public const int TOKEN_RETRIES = 50; public function __construct( string $appName, @@ -44,7 +44,7 @@ public function __construct( /** * Create a local editor * - * @param string $path Path of the file + * @param non-empty-string $path Path of the file * * @return DataResponse|DataResponse, array{}> * @@ -88,8 +88,8 @@ public function create(IUser $user, string $path): DataResponse { /** * Validate a local editor * - * @param string $path Path of the file - * @param string $token Token of the local editor + * @param non-empty-string $path Path of the file + * @param non-empty-string $token Token of the local editor * * @return DataResponse|DataResponse, array{}> * diff --git a/apps/files/lib/Controller/TransferOwnershipController.php b/apps/files/lib/Controller/TransferOwnershipController.php index 93039e6b7c9e7..d7351b3dd50dc 100644 --- a/apps/files/lib/Controller/TransferOwnershipController.php +++ b/apps/files/lib/Controller/TransferOwnershipController.php @@ -22,6 +22,7 @@ use OCP\Files\IHomeStorage; use OCP\Files\IRootFolder; use OCP\IRequest; +use OCP\IUser; use OCP\IUserManager; use OCP\Notification\IManager as NotificationManager; @@ -30,7 +31,6 @@ class TransferOwnershipController extends OCSController { public function __construct( string $appName, IRequest $request, - private string $userId, private NotificationManager $notificationManager, private ITimeFactory $timeFactory, private IJobList $jobList, @@ -54,14 +54,14 @@ public function __construct( * 403: Transferring ownership is not allowed */ #[NoAdminRequired] - public function transfer(string $recipient, string $path): DataResponse { + public function transfer(IUser $user, string $recipient, string $path): DataResponse { $recipientUser = $this->userManager->get($recipient); if ($recipientUser === null) { return new DataResponse([], Http::STATUS_BAD_REQUEST); } - $userRoot = $this->rootFolder->getUserFolder($this->userId); + $userRoot = $this->rootFolder->getUserFolder($user->getUID()); try { $node = $userRoot->get($path); @@ -69,15 +69,15 @@ public function transfer(string $recipient, string $path): DataResponse { return new DataResponse([], Http::STATUS_BAD_REQUEST); } - if ($node->getOwner()->getUID() !== $this->userId || !$node->getStorage()->instanceOfStorage(IHomeStorage::class)) { + if ($node->getOwner()->getUID() !== $user->getUID() || !$node->getStorage()->instanceOfStorage(IHomeStorage::class)) { return new DataResponse([], Http::STATUS_FORBIDDEN); } $transferOwnership = new TransferOwnershipEntity(); - $transferOwnership->setSourceUser($this->userId); - $transferOwnership->setTargetUser($recipient); - $transferOwnership->setFileId($node->getId()); - $transferOwnership->setNodeName($node->getName()); + $transferOwnership->sourceUser = $user->getUID(); + $transferOwnership->targetUser = $recipient; + $transferOwnership->fileId = $node->getId(); + $transferOwnership->nodeName = $node->getName(); $transferOwnership = $this->mapper->insert($transferOwnership); $notification = $this->notificationManager->createNotification(); @@ -85,11 +85,11 @@ public function transfer(string $recipient, string $path): DataResponse { ->setApp($this->appName) ->setDateTime($this->timeFactory->getDateTime()) ->setSubject('transferownershipRequest', [ - 'sourceUser' => $this->userId, + 'sourceUser' => $user->getUID(), 'targetUser' => $recipient, 'nodeName' => $node->getName(), ]) - ->setObject('transfer', (string)$transferOwnership->getId()); + ->setObject('transfer', (string)$transferOwnership->id); $this->notificationManager->notify($notification); @@ -108,19 +108,19 @@ public function transfer(string $recipient, string $path): DataResponse { * 404: Ownership transfer not found */ #[NoAdminRequired] - public function accept(int $id): DataResponse { + public function accept(IUser $user, int $id): DataResponse { try { $transferOwnership = $this->mapper->getById($id); } catch (DoesNotExistException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } - if ($transferOwnership->getTargetUser() !== $this->userId) { + if ($transferOwnership->targetUser !== $user->getUID()) { return new DataResponse([], Http::STATUS_FORBIDDEN); } $this->jobList->add(TransferOwnership::class, [ - 'id' => $transferOwnership->getId(), + 'id' => $transferOwnership->id, ]); $notification = $this->notificationManager->createNotification(); @@ -143,14 +143,14 @@ public function accept(int $id): DataResponse { * 404: Ownership transfer not found */ #[NoAdminRequired] - public function reject(int $id): DataResponse { + public function reject(IUser $user, int $id): DataResponse { try { $transferOwnership = $this->mapper->getById($id); } catch (DoesNotExistException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } - if ($transferOwnership->getTargetUser() !== $this->userId) { + if ($transferOwnership->targetUser !== $user->getUID()) { return new DataResponse([], Http::STATUS_FORBIDDEN); } diff --git a/apps/files/lib/Db/TransferOwnership.php b/apps/files/lib/Db/TransferOwnership.php index f1e89d5fcb888..b34f5e2d85439 100644 --- a/apps/files/lib/Db/TransferOwnership.php +++ b/apps/files/lib/Db/TransferOwnership.php @@ -9,35 +9,26 @@ namespace OCA\Files\Db; -use OCP\AppFramework\Db\Entity; +use OCP\AppFramework\ORM\Attribute\Column; +use OCP\AppFramework\ORM\Attribute\Entity; +use OCP\AppFramework\ORM\Attribute\Id; +use OCP\DB\Schema\ColumnType; -/** - * @method void setSourceUser(string $uid) - * @method string getSourceUser() - * @method void setTargetUser(string $uid) - * @method string getTargetUser() - * @method void setFileId(int $fileId) - * @method int getFileId() - * @method void setNodeName(string $name) - * @method string getNodeName() - */ -class TransferOwnership extends Entity { - /** @var string */ - protected $sourceUser; +#[Entity(name: 'user_transfer_owner')] +final class TransferOwnership { + #[Id] + #[Column(name: 'id', type: ColumnType::Bigint)] + public int $id; - /** @var string */ - protected $targetUser; + #[Column(name: 'source_user', type: ColumnType::String, length: 64)] + public string $sourceUser; - /** @var integer */ - protected $fileId; + #[Column(name: 'target_user', type: ColumnType::String, length: 64)] + public string $targetUser; - /** @var string */ - protected $nodeName; + #[Column(name: 'file_id', type: ColumnType::Bigint)] + public int $fileId; - public function __construct() { - $this->addType('sourceUser', 'string'); - $this->addType('targetUser', 'string'); - $this->addType('fileId', 'integer'); - $this->addType('nodeName', 'string'); - } + #[Column(name: 'node_name', type: ColumnType::String, length: 255)] + public string $nodeName; } diff --git a/apps/files/lib/Db/TransferOwnershipMapper.php b/apps/files/lib/Db/TransferOwnershipMapper.php index dc7b01ee4ba18..6885c51d02e4b 100644 --- a/apps/files/lib/Db/TransferOwnershipMapper.php +++ b/apps/files/lib/Db/TransferOwnershipMapper.php @@ -9,26 +9,15 @@ namespace OCA\Files\Db; -use OCP\AppFramework\Db\QBMapper; -use OCP\IDBConnection; +use OCP\AppFramework\ORM\Repository; /** - * @template-extends QBMapper + * @template-extends Repository */ -class TransferOwnershipMapper extends QBMapper { - public function __construct(IDBConnection $db) { - parent::__construct($db, 'user_transfer_owner', TransferOwnership::class); - } +class TransferOwnershipMapper extends Repository { + public const string entityClass = TransferOwnership::class; public function getById(int $id): TransferOwnership { - $qb = $this->db->getQueryBuilder(); - - $qb->select('*') - ->from($this->getTableName()) - ->where( - $qb->expr()->eq('id', $qb->createNamedParameter($id)) - ); - - return $this->findEntity($qb); + return $this->findOneBy(['id' => $id]); } } diff --git a/apps/files/lib/Notification/Notifier.php b/apps/files/lib/Notification/Notifier.php index a041d3ab9ad20..deba9fa15bc30 100644 --- a/apps/files/lib/Notification/Notifier.php +++ b/apps/files/lib/Notification/Notifier.php @@ -265,21 +265,21 @@ public function dismissNotification(INotification $notification): void { } if ($this->jobList->has(TransferOwnership::class, [ - 'id' => $transferOwnership->getId(), + 'id' => $transferOwnership->id, ])) { return; } $notification = $this->notificationManager->createNotification(); - $notification->setUser($transferOwnership->getSourceUser()) + $notification->setUser($transferOwnership->sourceUser) ->setApp('files') ->setDateTime($this->timeFactory->getDateTime()) ->setSubject('transferownershipRequestDenied', [ - 'sourceUser' => $transferOwnership->getSourceUser(), - 'targetUser' => $transferOwnership->getTargetUser(), - 'nodeName' => $transferOwnership->getNodeName() + 'sourceUser' => $transferOwnership->sourceUser, + 'targetUser' => $transferOwnership->targetUser, + 'nodeName' => $transferOwnership->nodeName ]) - ->setObject('transfer', (string)$transferOwnership->getId()); + ->setObject('transfer', (string)$transferOwnership->id); $this->notificationManager->notify($notification); $this->mapper->delete($transferOwnership); diff --git a/apps/files/tests/Db/TransferOwnershipMapperTest.php b/apps/files/tests/Db/TransferOwnershipMapperTest.php new file mode 100644 index 0000000000000..8fdf51e6cf363 --- /dev/null +++ b/apps/files/tests/Db/TransferOwnershipMapperTest.php @@ -0,0 +1,94 @@ +db->getQueryBuilder(); + $qb->delete($this->mapper->getTableName()) + ->where($qb->expr()->eq('source_user', $qb->createNamedParameter($this->sourceUser))); + $qb->executeStatement(); + } + + protected function setUp(): void { + parent::setUp(); + + $this->db = Server::get(IDBConnection::class); + $this->mapper = Server::get(TransferOwnershipMapper::class); + + $this->resetDB(); + } + + protected function tearDown(): void { + parent::tearDown(); + + $this->resetDB(); + } + + public function testInsertAndGetById(): void { + $entity = new TransferOwnership(); + $entity->sourceUser = $this->sourceUser; + $entity->targetUser = 'recipient123456'; + $entity->fileId = 42; + $entity->nodeName = 'welcome.txt'; + + $inserted = $this->mapper->insert($entity); + + $found = $this->mapper->getById($inserted->id); + + $this->assertSame($inserted->id, $found->id); + $this->assertSame($this->sourceUser, $found->sourceUser); + $this->assertSame('recipient123456', $found->targetUser); + $this->assertSame(42, $found->fileId); + $this->assertSame('welcome.txt', $found->nodeName); + } + + public function testGetByIdNotFound(): void { + $entity = new TransferOwnership(); + $entity->sourceUser = $this->sourceUser; + $entity->targetUser = 'recipient123456'; + $entity->fileId = 42; + $entity->nodeName = 'welcome.txt'; + + $inserted = $this->mapper->insert($entity); + $missingId = $inserted->id + 1000000; + + $this->mapper->delete($inserted); + + $this->expectException(DoesNotExistException::class); + $this->mapper->getById($missingId); + } + + public function testDelete(): void { + $entity = new TransferOwnership(); + $entity->sourceUser = $this->sourceUser; + $entity->targetUser = 'recipient123456'; + $entity->fileId = 42; + $entity->nodeName = 'welcome.txt'; + + $inserted = $this->mapper->insert($entity); + $this->mapper->delete($inserted); + + $this->expectException(DoesNotExistException::class); + $this->mapper->getById($inserted->id); + } +} From 0c6f6fbe9e367c17df6f9fb8fd37d4c35a3e04bd Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Thu, 20 Aug 2026 18:50:42 +0200 Subject: [PATCH 3/3] refactor: Use psalm:strict and rector:strict Signed-off-by: Carl Schwan --- .../lib/BackgroundJob/TransferOwnership.php | 31 ++++++---- .../Controller/OpenLocalEditorController.php | 19 +++--- .../TransferOwnershipController.php | 29 ++++----- apps/files/lib/Db/OpenLocalEditor.php | 3 + apps/files/lib/Db/OpenLocalEditorMapper.php | 2 +- apps/files/lib/Db/TransferOwnership.php | 3 + apps/files/lib/Db/TransferOwnershipMapper.php | 6 +- apps/files/lib/Notification/Notifier.php | 62 +++++++++++-------- .../tests/Db/OpenLocalEditorMapperTest.php | 6 +- .../tests/Db/TransferOwnershipMapperTest.php | 6 +- build/rector-strict.php | 10 +++ psalm-strict.xml | 10 +++ 12 files changed, 125 insertions(+), 62 deletions(-) diff --git a/apps/files/lib/BackgroundJob/TransferOwnership.php b/apps/files/lib/BackgroundJob/TransferOwnership.php index 55623a97a25cf..b9b8555d2dc38 100644 --- a/apps/files/lib/BackgroundJob/TransferOwnership.php +++ b/apps/files/lib/BackgroundJob/TransferOwnership.php @@ -23,21 +23,24 @@ use Psr\Log\LoggerInterface; use function ltrim; -class TransferOwnership extends QueuedJob { +final class TransferOwnership extends QueuedJob { public function __construct( ITimeFactory $timeFactory, - private IUserManager $userManager, - private OwnershipTransferService $transferService, - private LoggerInterface $logger, - private NotificationManager $notificationManager, - private TransferOwnershipMapper $mapper, - private IRootFolder $rootFolder, + private readonly IUserManager $userManager, + private readonly OwnershipTransferService $transferService, + private readonly LoggerInterface $logger, + private readonly NotificationManager $notificationManager, + private readonly TransferOwnershipMapper $mapper, + private readonly IRootFolder $rootFolder, ) { parent::__construct($timeFactory); } + /** + * @param array{id: int} $argument + */ #[\Override] - protected function run($argument) { + protected function run($argument): void { $id = $argument['id']; $transfer = $this->mapper->getById($id); @@ -53,7 +56,13 @@ protected function run($argument) { $this->failedNotication($transfer); return; } + $path = $userFolder->getRelativePath($node->getPath()); + if ($path === null) { + $this->logger->alert('Could not transfer ownership: Node not found'); + $this->failedNotication($transfer); + return; + } $sourceUserObject = $this->userManager->get($sourceUser); $destinationUserObject = $this->userManager->get($destinationUser); @@ -77,11 +86,11 @@ protected function run($argument) { ltrim($path, '/') ); $this->successNotification($transfer); - } catch (TransferOwnershipException $e) { + } catch (TransferOwnershipException $transferOwnershipException) { $this->logger->error( - $e->getMessage(), + $transferOwnershipException->getMessage(), [ - 'exception' => $e, + 'exception' => $transferOwnershipException, ], ); $this->failedNotication($transfer); diff --git a/apps/files/lib/Controller/OpenLocalEditorController.php b/apps/files/lib/Controller/OpenLocalEditorController.php index 470ff1d8f8a7d..6b4040dcbfcfe 100644 --- a/apps/files/lib/Controller/OpenLocalEditorController.php +++ b/apps/files/lib/Controller/OpenLocalEditorController.php @@ -25,18 +25,21 @@ use OCP\Security\ISecureRandom; use Psr\Log\LoggerInterface; -class OpenLocalEditorController extends OCSController { +final class OpenLocalEditorController extends OCSController { public const int TOKEN_LENGTH = 128; - public const int TOKEN_DURATION = 600; // 10 Minutes + + // 10 Minutes + public const int TOKEN_DURATION = 600; + public const int TOKEN_RETRIES = 50; public function __construct( string $appName, IRequest $request, - protected ITimeFactory $timeFactory, - protected OpenLocalEditorMapper $mapper, - protected ISecureRandom $secureRandom, - protected LoggerInterface $logger, + private readonly ITimeFactory $timeFactory, + private readonly OpenLocalEditorMapper $mapper, + private readonly ISecureRandom $secureRandom, + private readonly LoggerInterface $logger, ) { parent::__construct($appName, $request); } @@ -60,7 +63,7 @@ public function create(IUser $user, string $path): DataResponse { $entity->pathHash = $pathHash; $entity->expirationTime = $this->timeFactory->getTime() + self::TOKEN_DURATION; // Expire in 10 minutes - for ($i = 1; $i <= self::TOKEN_RETRIES; $i++) { + for ($i = 1; $i <= self::TOKEN_RETRIES; ++$i) { $token = $this->secureRandom->generate(self::TOKEN_LENGTH, ISecureRandom::CHAR_ALPHANUMERIC); $entity->token = $token; @@ -103,7 +106,7 @@ public function validate(IUser $user, string $path, string $token): DataResponse try { $entity = $this->mapper->verifyToken($user->getUID(), $pathHash, $token); - } catch (DoesNotExistException $e) { + } catch (DoesNotExistException) { $response = new DataResponse([], Http::STATUS_NOT_FOUND); $response->throttle(['userId' => $user->getUID(), 'pathHash' => $pathHash]); return $response; diff --git a/apps/files/lib/Controller/TransferOwnershipController.php b/apps/files/lib/Controller/TransferOwnershipController.php index d7351b3dd50dc..a4552465a75bd 100644 --- a/apps/files/lib/Controller/TransferOwnershipController.php +++ b/apps/files/lib/Controller/TransferOwnershipController.php @@ -26,17 +26,17 @@ use OCP\IUserManager; use OCP\Notification\IManager as NotificationManager; -class TransferOwnershipController extends OCSController { +final class TransferOwnershipController extends OCSController { public function __construct( string $appName, IRequest $request, - private NotificationManager $notificationManager, - private ITimeFactory $timeFactory, - private IJobList $jobList, - private TransferOwnershipMapper $mapper, - private IUserManager $userManager, - private IRootFolder $rootFolder, + private readonly NotificationManager $notificationManager, + private readonly ITimeFactory $timeFactory, + private readonly IJobList $jobList, + private readonly TransferOwnershipMapper $mapper, + private readonly IUserManager $userManager, + private readonly IRootFolder $rootFolder, ) { parent::__construct($appName, $request); } @@ -57,7 +57,7 @@ public function __construct( public function transfer(IUser $user, string $recipient, string $path): DataResponse { $recipientUser = $this->userManager->get($recipient); - if ($recipientUser === null) { + if (!$recipientUser instanceof IUser) { return new DataResponse([], Http::STATUS_BAD_REQUEST); } @@ -65,11 +65,12 @@ public function transfer(IUser $user, string $recipient, string $path): DataResp try { $node = $userRoot->get($path); - } catch (\Exception $e) { + } catch (\Exception) { return new DataResponse([], Http::STATUS_BAD_REQUEST); } - if ($node->getOwner()->getUID() !== $user->getUID() || !$node->getStorage()->instanceOfStorage(IHomeStorage::class)) { + $owner = $node->getOwner(); + if ($owner === null || $owner->getUID() !== $user->getUID() || !$node->getStorage()->instanceOfStorage(IHomeStorage::class)) { return new DataResponse([], Http::STATUS_FORBIDDEN); } @@ -99,7 +100,7 @@ public function transfer(IUser $user, string $recipient, string $path): DataResp /** * Accept an ownership transfer * - * @param int $id ID of the ownership transfer + * @param positive-int $id ID of the ownership transfer * * @return DataResponse, array{}> * @@ -111,7 +112,7 @@ public function transfer(IUser $user, string $recipient, string $path): DataResp public function accept(IUser $user, int $id): DataResponse { try { $transferOwnership = $this->mapper->getById($id); - } catch (DoesNotExistException $e) { + } catch (DoesNotExistException) { return new DataResponse([], Http::STATUS_NOT_FOUND); } @@ -134,7 +135,7 @@ public function accept(IUser $user, int $id): DataResponse { /** * Reject an ownership transfer * - * @param int $id ID of the ownership transfer + * @param positive-int $id ID of the ownership transfer * * @return DataResponse, array{}> * @@ -146,7 +147,7 @@ public function accept(IUser $user, int $id): DataResponse { public function reject(IUser $user, int $id): DataResponse { try { $transferOwnership = $this->mapper->getById($id); - } catch (DoesNotExistException $e) { + } catch (DoesNotExistException) { return new DataResponse([], Http::STATUS_NOT_FOUND); } diff --git a/apps/files/lib/Db/OpenLocalEditor.php b/apps/files/lib/Db/OpenLocalEditor.php index be5e434b03c90..1c058d72364e9 100644 --- a/apps/files/lib/Db/OpenLocalEditor.php +++ b/apps/files/lib/Db/OpenLocalEditor.php @@ -14,6 +14,9 @@ use OCP\AppFramework\ORM\Attribute\Id; use OCP\DB\Schema\ColumnType; +/** + * @psalm-suppress MissingConstructor ORM based hydration + */ #[Entity(name: 'open_local_editor')] final class OpenLocalEditor { #[Id] diff --git a/apps/files/lib/Db/OpenLocalEditorMapper.php b/apps/files/lib/Db/OpenLocalEditorMapper.php index 01dc9f1afd1d5..94f285b69cefe 100644 --- a/apps/files/lib/Db/OpenLocalEditorMapper.php +++ b/apps/files/lib/Db/OpenLocalEditorMapper.php @@ -16,7 +16,7 @@ /** * @template-extends Repository */ -class OpenLocalEditorMapper extends Repository { +final class OpenLocalEditorMapper extends Repository { public const string entityClass = OpenLocalEditor::class; /** diff --git a/apps/files/lib/Db/TransferOwnership.php b/apps/files/lib/Db/TransferOwnership.php index b34f5e2d85439..59ab4f0bff42a 100644 --- a/apps/files/lib/Db/TransferOwnership.php +++ b/apps/files/lib/Db/TransferOwnership.php @@ -14,6 +14,9 @@ use OCP\AppFramework\ORM\Attribute\Id; use OCP\DB\Schema\ColumnType; +/** + * @psalm-suppress MissingConstructor ORM based hydration + */ #[Entity(name: 'user_transfer_owner')] final class TransferOwnership { #[Id] diff --git a/apps/files/lib/Db/TransferOwnershipMapper.php b/apps/files/lib/Db/TransferOwnershipMapper.php index 6885c51d02e4b..b220bcd89e818 100644 --- a/apps/files/lib/Db/TransferOwnershipMapper.php +++ b/apps/files/lib/Db/TransferOwnershipMapper.php @@ -9,14 +9,18 @@ namespace OCA\Files\Db; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\ORM\Repository; /** * @template-extends Repository */ -class TransferOwnershipMapper extends Repository { +final class TransferOwnershipMapper extends Repository { public const string entityClass = TransferOwnership::class; + /** + * @throws DoesNotExistException + */ public function getById(int $id): TransferOwnership { return $this->findOneBy(['id' => $id]); } diff --git a/apps/files/lib/Notification/Notifier.php b/apps/files/lib/Notification/Notifier.php index deba9fa15bc30..6c7697f0f19ee 100644 --- a/apps/files/lib/Notification/Notifier.php +++ b/apps/files/lib/Notification/Notifier.php @@ -25,10 +25,10 @@ use OCP\Notification\INotifier; use OCP\Notification\UnknownNotificationException; -class Notifier implements INotifier, IDismissableNotifier { +final readonly class Notifier implements INotifier, IDismissableNotifier { public function __construct( - protected IFactory $l10nFactory, - protected IURLGenerator $urlGenerator, + private IFactory $l10nFactory, + private IURLGenerator $urlGenerator, private TransferOwnershipMapper $mapper, private IManager $notificationManager, private IUserManager $userManager, @@ -48,9 +48,7 @@ public function getName(): string { } /** - * @param INotification $notification * @param string $languageCode The code of the language that should be used to prepare the notification - * @return INotification * @throws UnknownNotificationException When the notification was not prepared by a notifier */ #[\Override] @@ -105,7 +103,9 @@ public function handleTransferownershipRequest(INotification $notification, stri IAction::TYPE_DELETE ); - $sourceUser = $this->getUser($param['sourceUser']); + $sourceUser = $this->getUser((string)$param['sourceUser']); + $targetUserId = (string)$param['targetUser']; + $nodeName = (string)$param['nodeName']; $notification->addParsedAction($approveAction) ->addParsedAction($disapproveAction) ->setRichSubject( @@ -122,8 +122,8 @@ public function handleTransferownershipRequest(INotification $notification, stri [ 'path' => [ 'type' => 'highlight', - 'id' => $param['targetUser'] . '::' . $param['nodeName'], - 'name' => $param['nodeName'], + 'id' => $targetUserId . '::' . $nodeName, + 'name' => $nodeName, ] ]); @@ -134,15 +134,17 @@ public function handleTransferOwnershipRequestDenied(INotification $notification $l = $this->l10nFactory->get('files', $languageCode); $param = $notification->getSubjectParameters(); - $targetUser = $this->getUser($param['targetUser']); + $targetUser = $this->getUser((string)$param['targetUser']); + $targetUserId = (string)$param['targetUser']; + $nodeName = (string)$param['nodeName']; $notification->setRichSubject($l->t('Ownership transfer denied')) ->setRichMessage( $l->t('Your ownership transfer of {path} was denied by {user}.'), [ 'path' => [ 'type' => 'highlight', - 'id' => $param['targetUser'] . '::' . $param['nodeName'], - 'name' => $param['nodeName'], + 'id' => $targetUserId . '::' . $nodeName, + 'name' => $nodeName, ], 'user' => [ 'type' => 'user', @@ -157,15 +159,17 @@ public function handleTransferOwnershipFailedSource(INotification $notification, $l = $this->l10nFactory->get('files', $languageCode); $param = $notification->getSubjectParameters(); - $targetUser = $this->getUser($param['targetUser']); + $targetUser = $this->getUser((string)$param['targetUser']); + $targetUserId = (string)$param['targetUser']; + $nodeName = (string)$param['nodeName']; $notification->setRichSubject($l->t('Ownership transfer failed')) ->setRichMessage( $l->t('Your ownership transfer of {path} to {user} failed.'), [ 'path' => [ 'type' => 'highlight', - 'id' => $param['targetUser'] . '::' . $param['nodeName'], - 'name' => $param['nodeName'], + 'id' => $targetUserId . '::' . $nodeName, + 'name' => $nodeName, ], 'user' => [ 'type' => 'user', @@ -180,15 +184,17 @@ public function handleTransferOwnershipFailedTarget(INotification $notification, $l = $this->l10nFactory->get('files', $languageCode); $param = $notification->getSubjectParameters(); - $sourceUser = $this->getUser($param['sourceUser']); + $sourceUser = $this->getUser((string)$param['sourceUser']); + $sourceUserId = (string)$param['sourceUser']; + $nodeName = (string)$param['nodeName']; $notification->setRichSubject($l->t('Ownership transfer failed')) ->setRichMessage( $l->t('The ownership transfer of {path} from {user} failed.'), [ 'path' => [ 'type' => 'highlight', - 'id' => $param['sourceUser'] . '::' . $param['nodeName'], - 'name' => $param['nodeName'], + 'id' => $sourceUserId . '::' . $nodeName, + 'name' => $nodeName, ], 'user' => [ 'type' => 'user', @@ -204,15 +210,17 @@ public function handleTransferOwnershipDoneSource(INotification $notification, s $l = $this->l10nFactory->get('files', $languageCode); $param = $notification->getSubjectParameters(); - $targetUser = $this->getUser($param['targetUser']); + $targetUser = $this->getUser((string)$param['targetUser']); + $targetUserId = (string)$param['targetUser']; + $nodeName = (string)$param['nodeName']; $notification->setRichSubject($l->t('Ownership transfer done')) ->setRichMessage( $l->t('Your ownership transfer of {path} to {user} has completed.'), [ 'path' => [ 'type' => 'highlight', - 'id' => $param['targetUser'] . '::' . $param['nodeName'], - 'name' => $param['nodeName'], + 'id' => $targetUserId . '::' . $nodeName, + 'name' => $nodeName, ], 'user' => [ 'type' => 'user', @@ -228,15 +236,17 @@ public function handleTransferOwnershipDoneTarget(INotification $notification, s $l = $this->l10nFactory->get('files', $languageCode); $param = $notification->getSubjectParameters(); - $sourceUser = $this->getUser($param['sourceUser']); + $sourceUser = $this->getUser((string)$param['sourceUser']); + $sourceUserId = (string)$param['sourceUser']; + $nodeName = (string)$param['nodeName']; $notification->setRichSubject($l->t('Ownership transfer done')) ->setRichMessage( $l->t('The ownership transfer of {path} from {user} has completed.'), [ 'path' => [ 'type' => 'highlight', - 'id' => $param['sourceUser'] . '::' . $param['nodeName'], - 'name' => $param['nodeName'], + 'id' => $sourceUserId . '::' . $nodeName, + 'name' => $nodeName, ], 'user' => [ 'type' => 'user', @@ -253,6 +263,7 @@ public function dismissNotification(INotification $notification): void { if ($notification->getApp() !== 'files') { throw new UnknownNotificationException('Unhandled app'); } + if ($notification->getSubject() !== 'transferownershipRequest') { throw new UnknownNotificationException('Unhandled notification type'); } @@ -260,7 +271,7 @@ public function dismissNotification(INotification $notification): void { // TODO: This should all be moved to a service that also the transferownershipController uses. try { $transferOwnership = $this->mapper->getById((int)$notification->getObjectId()); - } catch (DoesNotExistException $e) { + } catch (DoesNotExistException) { return; } @@ -285,11 +296,12 @@ public function dismissNotification(INotification $notification): void { $this->mapper->delete($transferOwnership); } - protected function getUser(string $userId): IUser { + private function getUser(string $userId): IUser { $user = $this->userManager->get($userId); if ($user instanceof IUser) { return $user; } + throw new \InvalidArgumentException('User not found'); } } diff --git a/apps/files/tests/Db/OpenLocalEditorMapperTest.php b/apps/files/tests/Db/OpenLocalEditorMapperTest.php index 739308a4822dc..f0fc560b8d2d1 100644 --- a/apps/files/tests/Db/OpenLocalEditorMapperTest.php +++ b/apps/files/tests/Db/OpenLocalEditorMapperTest.php @@ -17,9 +17,11 @@ use Test\TestCase; #[\PHPUnit\Framework\Attributes\Group(name: 'DB')] -class OpenLocalEditorMapperTest extends TestCase { +final class OpenLocalEditorMapperTest extends TestCase { private IDBConnection $db; + private OpenLocalEditorMapper $mapper; + private string $testUID = 'test123456'; private function resetDB(): void { @@ -29,6 +31,7 @@ private function resetDB(): void { $qb->executeStatement(); } + #[\Override] protected function setUp(): void { parent::setUp(); @@ -38,6 +41,7 @@ protected function setUp(): void { $this->resetDB(); } + #[\Override] protected function tearDown(): void { parent::tearDown(); diff --git a/apps/files/tests/Db/TransferOwnershipMapperTest.php b/apps/files/tests/Db/TransferOwnershipMapperTest.php index 8fdf51e6cf363..d9226acefa5b3 100644 --- a/apps/files/tests/Db/TransferOwnershipMapperTest.php +++ b/apps/files/tests/Db/TransferOwnershipMapperTest.php @@ -17,9 +17,11 @@ use Test\TestCase; #[\PHPUnit\Framework\Attributes\Group(name: 'DB')] -class TransferOwnershipMapperTest extends TestCase { +final class TransferOwnershipMapperTest extends TestCase { private IDBConnection $db; + private TransferOwnershipMapper $mapper; + private string $sourceUser = 'test123456'; private function resetDB(): void { @@ -29,6 +31,7 @@ private function resetDB(): void { $qb->executeStatement(); } + #[\Override] protected function setUp(): void { parent::setUp(); @@ -38,6 +41,7 @@ protected function setUp(): void { $this->resetDB(); } + #[\Override] protected function tearDown(): void { parent::tearDown(); diff --git a/build/rector-strict.php b/build/rector-strict.php index 7566ca1956a45..fbe0b35b34775 100644 --- a/build/rector-strict.php +++ b/build/rector-strict.php @@ -50,6 +50,16 @@ $nextcloudDir . '/lib/public/AppFramework/ORM', $nextcloudDir . '/lib/private/AppFramework/ORM', $nextcloudDir . '/apps/oauth2', + $nextcloudDir . '/apps/files/lib/Db/OpenLocalEditor.php', + $nextcloudDir . '/apps/files/lib/Db/OpenLocalEditorMapper.php', + $nextcloudDir . '/apps/files/lib/Controller/OpenLocalEditorController.php', + $nextcloudDir . '/apps/files/tests/Db/OpenLocalEditorMapperTest.php', + $nextcloudDir . '/apps/files/lib/Db/TransferOwnership.php', + $nextcloudDir . '/apps/files/lib/Db/TransferOwnershipMapper.php', + $nextcloudDir . '/apps/files/lib/Controller/TransferOwnershipController.php', + $nextcloudDir . '/apps/files/lib/BackgroundJob/TransferOwnership.php', + $nextcloudDir . '/apps/files/lib/Notification/Notifier.php', + $nextcloudDir . '/apps/files/tests/Db/TransferOwnershipMapperTest.php', ]) ->withAutoloadPaths([ // ensure rector properly autoload the public interfaces diff --git a/psalm-strict.xml b/psalm-strict.xml index 47b4de939a2b9..f1559b5fceb9f 100644 --- a/psalm-strict.xml +++ b/psalm-strict.xml @@ -56,6 +56,16 @@ + + + + + + + + + +