diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 0b8a0c0b8a3..b16a8d1e342 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -4306,4 +4306,13 @@ + + + + + + + + + diff --git a/lib/public/Comments/ICommentsManager.php b/lib/public/Comments/ICommentsManager.php index 1be8c73a336..651734ea1ff 100644 --- a/lib/public/Comments/ICommentsManager.php +++ b/lib/public/Comments/ICommentsManager.php @@ -438,6 +438,7 @@ interface ICommentsManager { * to consumers of the comments infrastructure * * @param \Closure $closure + * @return void * @since 11.0.0 */ public function registerEventHandler(\Closure $closure); @@ -447,6 +448,7 @@ interface ICommentsManager { * * @param string $type * @param \Closure $closure + * @return void * @throws \OutOfBoundsException * @since 11.0.0 * diff --git a/psalm.xml b/psalm.xml index f3b66f4f6e4..61fc300f101 100644 --- a/psalm.xml +++ b/psalm.xml @@ -54,6 +54,7 @@ + @@ -61,6 +62,7 @@ + @@ -71,6 +73,7 @@ + diff --git a/tests/lib/Comments/CommentTest.php b/tests/lib/Comments/CommentTest.php index 4a320666c83..670ccc69e12 100644 --- a/tests/lib/Comments/CommentTest.php +++ b/tests/lib/Comments/CommentTest.php @@ -11,6 +11,7 @@ use OC\Comments\Comment; use OCP\Comments\IComment; use OCP\Comments\IllegalIDChangeException; use OCP\Comments\MessageTooLongException; +use PHPUnit\Framework\Attributes\DataProvider; use Test\TestCase; class CommentTest extends TestCase { @@ -96,7 +97,7 @@ class CommentTest extends TestCase { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('simpleSetterProvider')] + #[DataProvider(methodName: 'simpleSetterProvider')] public function testSimpleSetterInvalidInput($field, $input): void { $this->expectException(\InvalidArgumentException::class); @@ -119,7 +120,7 @@ class CommentTest extends TestCase { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('roleSetterProvider')] + #[DataProvider(methodName: 'roleSetterProvider')] public function testSetRoleInvalidInput($role, $type, $id): void { $this->expectException(\InvalidArgumentException::class); @@ -204,13 +205,7 @@ class CommentTest extends TestCase { ]; } - /** - * - * @param string $message - * @param array $expectedMentions - * @param ?string $author - */ - #[\PHPUnit\Framework\Attributes\DataProvider('mentionsProvider')] + #[DataProvider(methodName: 'mentionsProvider')] public function testMentions(string $message, array $expectedMentions, ?string $author = null): void { $comment = new Comment(); $comment->setMessage($message); diff --git a/tests/lib/Comments/FakeManager.php b/tests/lib/Comments/FakeManager.php index ebee6519992..518e4801d0b 100644 --- a/tests/lib/Comments/FakeManager.php +++ b/tests/lib/Comments/FakeManager.php @@ -16,10 +16,12 @@ use OCP\IUser; * Class FakeManager */ class FakeManager implements ICommentsManager { - public function get($id) { + public function get($id): IComment { + throw new \Exception('Not implemented'); } - public function getTree($id, $limit = 0, $offset = 0) { + public function getTree($id, $limit = 0, $offset = 0): array { + return ['comment' => new Comment(), 'replies' => []]; } public function getForObject( @@ -28,7 +30,8 @@ class FakeManager implements ICommentsManager { $limit = 0, $offset = 0, ?\DateTime $notOlderThan = null, - ) { + ): array { + return []; } public function getForObjectSince( @@ -57,6 +60,7 @@ class FakeManager implements ICommentsManager { } public function getNumberOfCommentsForObject($objectType, $objectId, ?\DateTime $notOlderThan = null, $verb = ''): int { + return 0; } public function getNumberOfCommentsForObjects(string $objectType, array $objectIds, ?\DateTime $notOlderThan = null, string $verb = ''): array { @@ -67,10 +71,12 @@ class FakeManager implements ICommentsManager { return []; } - public function create($actorType, $actorId, $objectType, $objectId) { + public function create($actorType, $actorId, $objectType, $objectId): IComment { + return new Comment(); } - public function delete($id) { + public function delete($id): bool { + return false; } public function getReactionComment(int $parentId, string $actorType, string $actorId, string $reaction): IComment { @@ -89,47 +95,52 @@ class FakeManager implements ICommentsManager { return false; } - public function save(IComment $comment) { + public function save(IComment $comment): bool { + return false; } - public function deleteReferencesOfActor($actorType, $actorId) { + public function deleteReferencesOfActor($actorType, $actorId): bool { + return false; } - public function deleteCommentsAtObject($objectType, $objectId) { + public function deleteCommentsAtObject($objectType, $objectId): bool { + return false; } - public function setReadMark($objectType, $objectId, \DateTime $dateTime, IUser $user) { + public function setReadMark($objectType, $objectId, \DateTime $dateTime, IUser $user): bool { + return false; } - public function getReadMark($objectType, $objectId, IUser $user) { + public function getReadMark($objectType, $objectId, IUser $user): bool { + return false; } - public function deleteReadMarksFromUser(IUser $user) { + public function deleteReadMarksFromUser(IUser $user): bool { + return false; } - public function deleteReadMarksOnObject($objectType, $objectId) { + public function deleteReadMarksOnObject($objectType, $objectId): bool { + return false; } - public function registerEventHandler(\Closure $closure) { + public function registerEventHandler(\Closure $closure): void { } - public function registerDisplayNameResolver($type, \Closure $closure) { + public function registerDisplayNameResolver($type, \Closure $closure): void { } - public function resolveDisplayName($type, $id) { + public function resolveDisplayName($type, $id): string { + return ''; } - public function getNumberOfUnreadCommentsForFolder($folderId, IUser $user) { + public function getNumberOfUnreadCommentsForFolder($folderId, IUser $user): array { + return []; } public function getNumberOfUnreadCommentsForObjects(string $objectType, array $objectIds, IUser $user, $verb = ''): array { return []; } - - public function getActorsInTree($id) { - } - public function load(): void { } diff --git a/tests/lib/Comments/ManagerTest.php b/tests/lib/Comments/ManagerTest.php index d88e062a8ec..d0cb1e00fe8 100644 --- a/tests/lib/Comments/ManagerTest.php +++ b/tests/lib/Comments/ManagerTest.php @@ -26,18 +26,19 @@ use OCP\IInitialStateService; use OCP\IUser; use OCP\IUserManager; use OCP\Server; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\TestCase; /** * Class ManagerTest */ -#[\PHPUnit\Framework\Attributes\Group('DB')] +#[Group(name: 'DB')] class ManagerTest extends TestCase { - /** @var IDBConnection */ - private $connection; - /** @var \PHPUnit\Framework\MockObject\MockObject|IRootFolder */ - private $rootFolder; + private IDBConnection $connection; + private IRootFolder&MockObject $rootFolder; protected function setUp(): void { parent::setUp(); @@ -45,13 +46,15 @@ class ManagerTest extends TestCase { $this->connection = Server::get(IDBConnection::class); $this->rootFolder = $this->createMock(IRootFolder::class); + /** @psalm-suppress DeprecatedMethod */ $sql = $this->connection->getDatabasePlatform()->getTruncateTableSQL('`*PREFIX*comments`'); $this->connection->prepare($sql)->execute(); + /** @psalm-suppress DeprecatedMethod */ $sql = $this->connection->getDatabasePlatform()->getTruncateTableSQL('`*PREFIX*reactions`'); $this->connection->prepare($sql)->execute(); } - protected function addDatabaseEntry($parentId, $topmostParentId, $creationDT = null, $latestChildDT = null, $objectId = null, $expireDate = null) { + protected function addDatabaseEntry(?string $parentId, ?string $topmostParentId, ?\DateTimeInterface $creationDT = null, ?\DateTimeInterface $latestChildDT = null, $objectId = null, ?\DateTimeInterface $expireDate = null): string { $creationDT ??= new \DateTime(); $latestChildDT ??= new \DateTime('yesterday'); $objectId ??= 'file64'; @@ -77,10 +80,11 @@ class ManagerTest extends TestCase { ]) ->executeStatement(); - return $qb->getLastInsertId(); + return (string)$qb->getLastInsertId(); } - protected function getManager() { + protected function getManager(): Manager { + /** @psalm-suppress DeprecatedInterface No way around at the moment */ return new Manager( $this->connection, $this->createMock(LoggerInterface::class), @@ -115,7 +119,7 @@ class ManagerTest extends TestCase { $creationDT = new \DateTime('yesterday'); $latestChildDT = new \DateTime(); - $qb = Server::get(IDBConnection::class)->getQueryBuilder(); + $qb = $this->connection->getQueryBuilder(); $qb ->insert('comments') ->values([ @@ -155,7 +159,6 @@ class ManagerTest extends TestCase { $this->assertEquals(['last_edit_actor_id' => 'admin'], $comment->getMetaData()); } - public function testGetTreeNotFound(): void { $this->expectException(NotFoundException::class); @@ -163,7 +166,6 @@ class ManagerTest extends TestCase { $manager->getTree('22'); } - public function testGetTreeNotFoundInvalidIpnut(): void { $this->expectException(\InvalidArgumentException::class); @@ -172,7 +174,7 @@ class ManagerTest extends TestCase { } public function testGetTree(): void { - $headId = $this->addDatabaseEntry(0, 0); + $headId = $this->addDatabaseEntry('0', '0'); $this->addDatabaseEntry($headId, $headId, new \DateTime('-3 hours')); $this->addDatabaseEntry($headId, $headId, new \DateTime('-2 hours')); @@ -184,7 +186,7 @@ class ManagerTest extends TestCase { // Verifying the root comment $this->assertArrayHasKey('comment', $tree); $this->assertInstanceOf(IComment::class, $tree['comment']); - $this->assertSame((string)$headId, $tree['comment']->getId()); + $this->assertSame($headId, $tree['comment']->getId()); $this->assertArrayHasKey('replies', $tree); $this->assertCount(3, $tree['replies']); @@ -198,7 +200,7 @@ class ManagerTest extends TestCase { } public function testGetTreeNoReplies(): void { - $id = $this->addDatabaseEntry(0, 0); + $id = $this->addDatabaseEntry('0', '0'); $manager = $this->getManager(); $tree = $manager->getTree($id); @@ -206,13 +208,13 @@ class ManagerTest extends TestCase { // Verifying the root comment $this->assertArrayHasKey('comment', $tree); $this->assertInstanceOf(IComment::class, $tree['comment']); - $this->assertSame((string)$id, $tree['comment']->getId()); + $this->assertSame($id, $tree['comment']->getId()); $this->assertArrayHasKey('replies', $tree); $this->assertCount(0, $tree['replies']); } public function testGetTreeWithLimitAndOffset(): void { - $headId = $this->addDatabaseEntry(0, 0); + $headId = $this->addDatabaseEntry('0', '0'); $this->addDatabaseEntry($headId, $headId, new \DateTime('-3 hours')); $this->addDatabaseEntry($headId, $headId, new \DateTime('-2 hours')); @@ -222,19 +224,19 @@ class ManagerTest extends TestCase { $manager = $this->getManager(); for ($offset = 0; $offset < 3; $offset += 2) { - $tree = $manager->getTree((string)$headId, 2, $offset); + $tree = $manager->getTree($headId, 2, $offset); // Verifying the root comment $this->assertArrayHasKey('comment', $tree); $this->assertInstanceOf(IComment::class, $tree['comment']); - $this->assertSame((string)$headId, $tree['comment']->getId()); + $this->assertSame($headId, $tree['comment']->getId()); $this->assertArrayHasKey('replies', $tree); $this->assertCount(2, $tree['replies']); // one level deep foreach ($tree['replies'] as $reply) { $this->assertInstanceOf(IComment::class, $reply['comment']); - $this->assertSame((string)$idToVerify, $reply['comment']->getId()); + $this->assertSame((string)$idToVerify, (string)$reply['comment']->getId()); $this->assertCount(0, $reply['replies']); $idToVerify--; } @@ -242,7 +244,7 @@ class ManagerTest extends TestCase { } public function testGetForObject(): void { - $this->addDatabaseEntry(0, 0); + $this->addDatabaseEntry('0', '0'); $manager = $this->getManager(); $comments = $manager->getForObject('files', 'file64'); @@ -254,13 +256,13 @@ class ManagerTest extends TestCase { } public function testGetForObjectWithLimitAndOffset(): void { - $this->addDatabaseEntry(0, 0, new \DateTime('-6 hours')); - $this->addDatabaseEntry(0, 0, new \DateTime('-5 hours')); - $this->addDatabaseEntry(1, 1, new \DateTime('-4 hours')); - $this->addDatabaseEntry(0, 0, new \DateTime('-3 hours')); - $this->addDatabaseEntry(2, 2, new \DateTime('-2 hours')); - $this->addDatabaseEntry(2, 2, new \DateTime('-1 hours')); - $idToVerify = $this->addDatabaseEntry(3, 1, new \DateTime()); + $this->addDatabaseEntry('0', '0', new \DateTime('-6 hours')); + $this->addDatabaseEntry('0', '0', new \DateTime('-5 hours')); + $this->addDatabaseEntry('1', '1', new \DateTime('-4 hours')); + $this->addDatabaseEntry('0', '0', new \DateTime('-3 hours')); + $this->addDatabaseEntry('2', '2', new \DateTime('-2 hours')); + $this->addDatabaseEntry('2', '2', new \DateTime('-1 hours')); + $idToVerify = $this->addDatabaseEntry('3', '1', new \DateTime()); $manager = $this->getManager(); $offset = 0; @@ -279,27 +281,27 @@ class ManagerTest extends TestCase { } public function testGetForObjectWithDateTimeConstraint(): void { - $this->addDatabaseEntry(0, 0, new \DateTime('-6 hours')); - $this->addDatabaseEntry(0, 0, new \DateTime('-5 hours')); - $id1 = $this->addDatabaseEntry(0, 0, new \DateTime('-3 hours')); - $id2 = $this->addDatabaseEntry(2, 2, new \DateTime('-2 hours')); + $this->addDatabaseEntry('0', '0', new \DateTime('-6 hours')); + $this->addDatabaseEntry('0', '0', new \DateTime('-5 hours')); + $id1 = $this->addDatabaseEntry('0', '0', new \DateTime('-3 hours')); + $id2 = $this->addDatabaseEntry('2', '2', new \DateTime('-2 hours')); $manager = $this->getManager(); $comments = $manager->getForObject('files', 'file64', 0, 0, new \DateTime('-4 hours')); $this->assertCount(2, $comments); - $this->assertSame((string)$id2, $comments[0]->getId()); - $this->assertSame((string)$id1, $comments[1]->getId()); + $this->assertSame($id2, $comments[0]->getId()); + $this->assertSame($id1, $comments[1]->getId()); } public function testGetForObjectWithLimitAndOffsetAndDateTimeConstraint(): void { - $this->addDatabaseEntry(0, 0, new \DateTime('-7 hours')); - $this->addDatabaseEntry(0, 0, new \DateTime('-6 hours')); - $this->addDatabaseEntry(1, 1, new \DateTime('-5 hours')); - $this->addDatabaseEntry(0, 0, new \DateTime('-3 hours')); - $this->addDatabaseEntry(2, 2, new \DateTime('-2 hours')); - $this->addDatabaseEntry(2, 2, new \DateTime('-1 hours')); - $idToVerify = $this->addDatabaseEntry(3, 1, new \DateTime()); + $this->addDatabaseEntry('0', '0', new \DateTime('-7 hours')); + $this->addDatabaseEntry('0', '0', new \DateTime('-6 hours')); + $this->addDatabaseEntry('1', '1', new \DateTime('-5 hours')); + $this->addDatabaseEntry('0', '0', new \DateTime('-3 hours')); + $this->addDatabaseEntry('2', '2', new \DateTime('-2 hours')); + $this->addDatabaseEntry('2', '2', new \DateTime('-1 hours')); + $idToVerify = $this->addDatabaseEntry('3', '1', new \DateTime()); $manager = $this->getManager(); $offset = 0; @@ -320,7 +322,7 @@ class ManagerTest extends TestCase { public function testGetNumberOfCommentsForObject(): void { for ($i = 1; $i < 5; $i++) { - $this->addDatabaseEntry(0, 0); + $this->addDatabaseEntry('0', '0'); } $manager = $this->getManager(); @@ -351,11 +353,11 @@ class ManagerTest extends TestCase { // 2 comments for 1112 with no read marker // 1 comment for 1113 before read marker // 1 comment for 1114 with no read marker - $this->addDatabaseEntry(0, 0, null, null, $fileIds[1]); + $this->addDatabaseEntry('0', '0', null, null, $fileIds[1]); for ($i = 0; $i < 4; $i++) { - $this->addDatabaseEntry(0, 0, null, null, $fileIds[$i]); + $this->addDatabaseEntry('0', '0', null, null, $fileIds[$i]); } - $this->addDatabaseEntry(0, 0, (new \DateTime())->modify('-2 days'), null, $fileIds[0]); + $this->addDatabaseEntry('0', '0', (new \DateTime())->modify('-2 days'), null, $fileIds[0]); /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ $user = $this->createMock(IUser::class); $user->expects($this->any()) @@ -375,24 +377,24 @@ class ManagerTest extends TestCase { ], $amount); } - #[\PHPUnit\Framework\Attributes\DataProvider('dataGetForObjectSince')] + #[DataProvider(methodName: 'dataGetForObjectSince')] public function testGetForObjectSince(?int $lastKnown, string $order, int $limit, int $resultFrom, int $resultTo): void { $ids = []; - $ids[] = $this->addDatabaseEntry(0, 0); - $ids[] = $this->addDatabaseEntry(0, 0); - $ids[] = $this->addDatabaseEntry(0, 0); - $ids[] = $this->addDatabaseEntry(0, 0); - $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry('0', '0'); + $ids[] = $this->addDatabaseEntry('0', '0'); + $ids[] = $this->addDatabaseEntry('0', '0'); + $ids[] = $this->addDatabaseEntry('0', '0'); + $ids[] = $this->addDatabaseEntry('0', '0'); $manager = $this->getManager(); - $comments = $manager->getForObjectSince('files', 'file64', ($lastKnown === null ? 0 : $ids[$lastKnown]), $order, $limit); + $comments = $manager->getForObjectSince('files', 'file64', ($lastKnown === null ? 0 : (int)$ids[$lastKnown]), $order, $limit); $expected = array_slice($ids, $resultFrom, $resultTo - $resultFrom + 1); if ($order === 'desc') { $expected = array_reverse($expected); } - $this->assertSame($expected, array_map(static fn (IComment $c): int => (int)$c->getId(), $comments)); + $this->assertSame($expected, array_map(static fn (IComment $c): string => $c->getId(), $comments)); } public static function dataGetForObjectSince(): array { @@ -421,7 +423,7 @@ class ManagerTest extends TestCase { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('invalidCreateArgsProvider')] + #[DataProvider(methodName: 'invalidCreateArgsProvider')] public function testCreateCommentInvalidArguments(string|int $aType, string|int $aId, string|int $oType, string|int $oId): void { $this->expectException(\InvalidArgumentException::class); @@ -458,7 +460,7 @@ class ManagerTest extends TestCase { $done = $manager->delete(''); $this->assertFalse($done); - $id = (string)$this->addDatabaseEntry(0, 0); + $id = $this->addDatabaseEntry('0', '0'); $comment = $manager->get($id); $this->assertInstanceOf(IComment::class, $comment); $done = $manager->delete($id); @@ -466,7 +468,7 @@ class ManagerTest extends TestCase { $manager->get($id); } - #[\PHPUnit\Framework\Attributes\DataProvider('providerTestSave')] + #[DataProvider(methodName: 'providerTestSave')] public function testSave(string $message, string $actorId, string $verb, ?string $parentId, ?string $id = ''): IComment { $manager = $this->getManager(); $comment = new Comment(); @@ -573,7 +575,7 @@ class ManagerTest extends TestCase { } public function testSaveAsChild(): void { - $id = (string)$this->addDatabaseEntry(0, 0); + $id = $this->addDatabaseEntry('0', '0'); $manager = $this->getManager(); @@ -606,7 +608,7 @@ class ManagerTest extends TestCase { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('invalidActorArgsProvider')] + #[DataProvider(methodName: 'invalidActorArgsProvider')] public function testDeleteReferencesOfActorInvalidInput(string|int $type, string|int $id): void { $this->expectException(\InvalidArgumentException::class); @@ -616,14 +618,14 @@ class ManagerTest extends TestCase { public function testDeleteReferencesOfActor(): void { $ids = []; - $ids[] = $this->addDatabaseEntry(0, 0); - $ids[] = $this->addDatabaseEntry(0, 0); - $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry('0', '0'); + $ids[] = $this->addDatabaseEntry('0', '0'); + $ids[] = $this->addDatabaseEntry('0', '0'); $manager = $this->getManager(); // just to make sure they are really set, with correct actor data - $comment = $manager->get((string)$ids[1]); + $comment = $manager->get($ids[1]); $this->assertSame('users', $comment->getActorType()); $this->assertSame('alice', $comment->getActorId()); @@ -631,7 +633,7 @@ class ManagerTest extends TestCase { $this->assertTrue($wasSuccessful); foreach ($ids as $id) { - $comment = $manager->get((string)$id); + $comment = $manager->get($id); $this->assertSame(ICommentsManager::DELETED_USER, $comment->getActorType()); $this->assertSame(ICommentsManager::DELETED_USER, $comment->getActorId()); } @@ -671,7 +673,7 @@ class ManagerTest extends TestCase { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('invalidObjectArgsProvider')] + #[DataProvider(methodName: 'invalidObjectArgsProvider')] public function testDeleteCommentsAtObjectInvalidInput(string|int $type, string|int $id): void { $this->expectException(\InvalidArgumentException::class); @@ -681,14 +683,14 @@ class ManagerTest extends TestCase { public function testDeleteCommentsAtObject(): void { $ids = []; - $ids[] = $this->addDatabaseEntry(0, 0); - $ids[] = $this->addDatabaseEntry(0, 0); - $ids[] = $this->addDatabaseEntry(0, 0); + $ids[] = $this->addDatabaseEntry('0', '0'); + $ids[] = $this->addDatabaseEntry('0', '0'); + $ids[] = $this->addDatabaseEntry('0', '0'); $manager = $this->getManager(); // just to make sure they are really set, with correct actor data - $comment = $manager->get((string)$ids[1]); + $comment = $manager->get($ids[1]); $this->assertSame('files', $comment->getObjectType()); $this->assertSame('file64', $comment->getObjectId()); @@ -698,7 +700,7 @@ class ManagerTest extends TestCase { $verified = 0; foreach ($ids as $id) { try { - $manager->get((string)$id); + $manager->get($id); } catch (NotFoundException) { $verified++; } @@ -713,13 +715,14 @@ class ManagerTest extends TestCase { public function testDeleteCommentsExpiredAtObjectTypeAndId(): void { $ids = []; - $ids[] = $this->addDatabaseEntry(0, 0, null, null, null, new \DateTime('+2 hours')); - $ids[] = $this->addDatabaseEntry(0, 0, null, null, null, new \DateTime('+2 hours')); - $ids[] = $this->addDatabaseEntry(0, 0, null, null, null, new \DateTime('+2 hours')); - $ids[] = $this->addDatabaseEntry(0, 0, null, null, null, new \DateTime('-2 hours')); - $ids[] = $this->addDatabaseEntry(0, 0, null, null, null, new \DateTime('-2 hours')); - $ids[] = $this->addDatabaseEntry(0, 0, null, null, null, new \DateTime('-2 hours')); - + $ids[] = $this->addDatabaseEntry('0', '0', null, null, null, new \DateTime('+2 hours')); + $ids[] = $this->addDatabaseEntry('0', '0', null, null, null, new \DateTime('+2 hours')); + $ids[] = $this->addDatabaseEntry('0', '0', null, null, null, new \DateTime('+2 hours')); + $ids[] = $this->addDatabaseEntry('0', '0', null, null, null, new \DateTime('-2 hours')); + $ids[] = $this->addDatabaseEntry('0', '0', null, null, null, new \DateTime('-2 hours')); + $ids[] = $this->addDatabaseEntry('0', '0', null, null, null, new \DateTime('-2 hours')); + + /** @psalm-suppress DeprecatedInterface No way around at the moment */ $manager = new Manager( $this->connection, $this->createMock(LoggerInterface::class), @@ -732,7 +735,7 @@ class ManagerTest extends TestCase { ); // just to make sure they are really set, with correct actor data - $comment = $manager->get((string)$ids[1]); + $comment = $manager->get($ids[1]); $this->assertSame('files', $comment->getObjectType()); $this->assertSame('file64', $comment->getObjectId()); @@ -743,7 +746,7 @@ class ManagerTest extends TestCase { $exists = 0; foreach ($ids as $id) { try { - $manager->get((string)$id); + $manager->get($id); $exists++; } catch (NotFoundException) { $deleted++; @@ -760,13 +763,14 @@ class ManagerTest extends TestCase { public function testDeleteCommentsExpiredAtObjectType(): void { $ids = []; - $ids[] = $this->addDatabaseEntry(0, 0, null, null, 'file1', new \DateTime('-2 hours')); - $ids[] = $this->addDatabaseEntry(0, 0, null, null, 'file2', new \DateTime('-2 hours')); - $ids[] = $this->addDatabaseEntry(0, 0, null, null, 'file3', new \DateTime('-2 hours')); - $ids[] = $this->addDatabaseEntry(0, 0, null, null, 'file3', new \DateTime()); - $ids[] = $this->addDatabaseEntry(0, 0, null, null, 'file3', new \DateTime()); - $ids[] = $this->addDatabaseEntry(0, 0, null, null, 'file3', new \DateTime()); - + $ids[] = $this->addDatabaseEntry('0', '0', null, null, 'file1', new \DateTime('-2 hours')); + $ids[] = $this->addDatabaseEntry('0', '0', null, null, 'file2', new \DateTime('-2 hours')); + $ids[] = $this->addDatabaseEntry('0', '0', null, null, 'file3', new \DateTime('-2 hours')); + $ids[] = $this->addDatabaseEntry('0', '0', null, null, 'file3', new \DateTime()); + $ids[] = $this->addDatabaseEntry('0', '0', null, null, 'file3', new \DateTime()); + $ids[] = $this->addDatabaseEntry('0', '0', null, null, 'file3', new \DateTime()); + + /** @psalm-suppress DeprecatedInterface No way around at the moment */ $manager = new Manager( $this->connection, $this->createMock(LoggerInterface::class), @@ -785,7 +789,7 @@ class ManagerTest extends TestCase { $exists = 0; foreach ($ids as $id) { try { - $manager->get((string)$id); + $manager->get($id); $exists++; } catch (NotFoundException) { $deleted++; @@ -838,7 +842,6 @@ class ManagerTest extends TestCase { } public function testReadMarkDeleteUser(): void { - /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ $user = $this->createMock(IUser::class); $user->expects($this->any()) ->method('getUID') @@ -856,7 +859,6 @@ class ManagerTest extends TestCase { } public function testReadMarkDeleteObject(): void { - /** @var IUser|\PHPUnit\Framework\MockObject\MockObject $user */ $user = $this->createMock(IUser::class); $user->expects($this->any()) ->method('getUID') @@ -874,10 +876,12 @@ class ManagerTest extends TestCase { } public function testSendEvent(): void { + /** @psalm-suppress DeprecatedInterface Test for deprecated interface */ $handler1 = $this->createMock(ICommentsEventHandler::class); $handler1->expects($this->exactly(4)) ->method('handle'); + /** @psalm-suppress DeprecatedInterface Test for deprecated interface */ $handler2 = $this->createMock(ICommentsEventHandler::class); $handler1->expects($this->exactly(4)) ->method('handle'); @@ -932,35 +936,18 @@ class ManagerTest extends TestCase { $manager = $this->getManager(); - $planetClosure = function ($name) { - return ucfirst($name); - }; + $planetClosure = static fn (string $name): string => ucfirst($name); $manager->registerDisplayNameResolver('planet', $planetClosure); $manager->registerDisplayNameResolver('planet', $planetClosure); } - public function testRegisterResolverInvalidType(): void { - $this->expectException(\InvalidArgumentException::class); - - $manager = $this->getManager(); - - $planetClosure = function ($name) { - return ucfirst($name); - }; - $manager->registerDisplayNameResolver(1337, $planetClosure); - } - - public function testResolveDisplayNameUnregisteredType(): void { $this->expectException(\OutOfBoundsException::class); $manager = $this->getManager(); - $planetClosure = function ($name) { - return ucfirst($name); - }; - + $planetClosure = static fn (string $name): string => ucfirst($name); $manager->registerDisplayNameResolver('planet', $planetClosure); $manager->resolveDisplayName('galaxy', 'sombrero'); } @@ -968,34 +955,18 @@ class ManagerTest extends TestCase { public function testResolveDisplayNameDirtyResolver(): void { $manager = $this->getManager(); - $planetClosure = function () { - return null; - }; - + $planetClosure = static fn (): null => null; $manager->registerDisplayNameResolver('planet', $planetClosure); $this->assertIsString($manager->resolveDisplayName('planet', 'neptune')); } - public function testResolveDisplayNameInvalidType(): void { - - $manager = $this->getManager(); - - $planetClosure = function () { - return null; - }; - - $manager->registerDisplayNameResolver('planet', $planetClosure); - $this->expectException(\InvalidArgumentException::class); - $this->assertIsString($manager->resolveDisplayName(1337, 'neptune')); - } - private function skipIfNotSupport4ByteUTF(): void { if (!$this->getManager()->supportReactions()) { $this->markTestSkipped('MySQL doesn\'t support 4 byte UTF-8'); } } - #[\PHPUnit\Framework\Attributes\DataProvider('providerTestReactionAddAndDelete')] + #[DataProvider(methodName: 'providerTestReactionAddAndDelete')] public function testReactionAddAndDelete(array $comments, array $reactionsExpected): void { $this->skipIfNotSupport4ByteUTF(); $manager = $this->getManager(); @@ -1067,7 +1038,7 @@ class ManagerTest extends TestCase { [$message, $actorId, $verb, $parentText] = $comment; $parentId = null; if ($parentText) { - $parentId = (string)$comments[$parentText]->getId(); + $parentId = $comments[$parentText]->getId(); } $id = ''; if ($verb === 'reaction_deleted') { @@ -1080,7 +1051,7 @@ class ManagerTest extends TestCase { return $comments; } - #[\PHPUnit\Framework\Attributes\DataProvider('providerTestRetrieveAllReactions')] + #[DataProvider(methodName: 'providerTestRetrieveAllReactions')] public function testRetrieveAllReactions(array $comments, array $expected): void { $this->skipIfNotSupport4ByteUTF(); $manager = $this->getManager(); @@ -2342,7 +2313,7 @@ class ManagerTest extends TestCase { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('providerTestRetrieveAllReactionsWithSpecificReaction')] + #[DataProvider(methodName: 'providerTestRetrieveAllReactionsWithSpecificReaction')] public function testRetrieveAllReactionsWithSpecificReaction(array $comments, string $reaction, array $expected): void { $this->skipIfNotSupport4ByteUTF(); $manager = $this->getManager(); @@ -2395,7 +2366,7 @@ class ManagerTest extends TestCase { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('providerTestGetReactionComment')] + #[DataProvider(methodName: 'providerTestGetReactionComment')] public function testGetReactionComment(array $comments, array $expected, bool $notFound): void { $this->skipIfNotSupport4ByteUTF(); $manager = $this->getManager(); @@ -2462,7 +2433,7 @@ class ManagerTest extends TestCase { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('providerTestReactionMessageSize')] + #[DataProvider(methodName: 'providerTestReactionMessageSize')] public function testReactionMessageSize(string $reactionString, bool $valid): void { $this->skipIfNotSupport4ByteUTF(); if (!$valid) { @@ -2491,7 +2462,7 @@ class ManagerTest extends TestCase { ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('providerTestReactionsSummarizeOrdered')] + #[DataProvider(methodName: 'providerTestReactionsSummarizeOrdered')] public function testReactionsSummarizeOrdered(array $comments, array $expected, bool $isFullMatch): void { $this->skipIfNotSupport4ByteUTF(); $manager = $this->getManager(); diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index 1b387df0eb5..6fb5bc309ed 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -8,8 +8,6 @@ namespace Test; -use DOMDocument; -use DOMNode; use OC\App\AppStore\Fetcher\AppFetcher; use OC\Command\QueueBus; use OC\Files\AppData\Factory; @@ -23,15 +21,12 @@ use OC\Files\ObjectStore\PrimaryObjectStoreConfig; use OC\Files\SetupManager; use OC\Files\View; use OC\Installer; -use OC\Template\Base; use OC\Updater; -use OCP\AppFramework\QueryException; use OCP\Command\IBus; use OCP\DB\QueryBuilder\IQueryBuilder; -use OCP\Defaults; +use OCP\Files\IRootFolder; use OCP\IConfig; use OCP\IDBConnection; -use OCP\IL10N; use OCP\IUserManager; use OCP\IUserSession; use OCP\Lock\ILockingProvider; @@ -39,69 +34,39 @@ use OCP\Lock\LockedException; use OCP\Security\ISecureRandom; use OCP\Server; use PHPUnit\Framework\Attributes\Group; - -if (version_compare(\PHPUnit\Runner\Version::id(), 10, '>=')) { - trait OnNotSuccessfulTestTrait { - protected function onNotSuccessfulTest(\Throwable $t): never { - $this->restoreAllServices(); - - // restore database connection - if (!$this->IsDatabaseAccessAllowed()) { - \OC::$server->registerService(IDBConnection::class, function () { - return self::$realDatabase; - }); - } - - parent::onNotSuccessfulTest($t); - } - } -} else { - trait OnNotSuccessfulTestTrait { - protected function onNotSuccessfulTest(\Throwable $t): void { - $this->restoreAllServices(); - - // restore database connection - if (!$this->IsDatabaseAccessAllowed()) { - \OC::$server->registerService(IDBConnection::class, function () { - return self::$realDatabase; - }); - } - - parent::onNotSuccessfulTest($t); - } - } -} +use Psr\Container\ContainerExceptionInterface; abstract class TestCase extends \PHPUnit\Framework\TestCase { - /** @var \OC\Command\QueueBus */ - private $commandBus; + private QueueBus $commandBus; - /** @var IDBConnection */ - protected static $realDatabase = null; + protected static ?IDBConnection $realDatabase = null; + private static bool $wasDatabaseAllowed = false; + protected array $services = []; - /** @var bool */ - private static $wasDatabaseAllowed = false; + protected function onNotSuccessfulTest(\Throwable $t): never { + $this->restoreAllServices(); - /** @var array */ - protected $services = []; + // restore database connection + if (!$this->IsDatabaseAccessAllowed()) { + \OC::$server->registerService(IDBConnection::class, function () { + return self::$realDatabase; + }); + } - use OnNotSuccessfulTestTrait; + parent::onNotSuccessfulTest($t); + } - /** - * @param string $name - * @param mixed $newService - * @return bool - */ - public function overwriteService(string $name, $newService): bool { + public function overwriteService(string $name, mixed $newService): bool { if (isset($this->services[$name])) { return false; } try { $this->services[$name] = Server::get($name); - } catch (QueryException $e) { + } catch (ContainerExceptionInterface $e) { $this->services[$name] = false; } + /** @psalm-suppress InternalMethod */ $container = \OC::$server->getAppContainerForService($name); $container = $container ?? \OC::$server; @@ -112,14 +77,11 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { return true; } - /** - * @param string $name - * @return bool - */ public function restoreService(string $name): bool { if (isset($this->services[$name])) { $oldService = $this->services[$name]; + /** @psalm-suppress InternalMethod */ $container = \OC::$server->getAppContainerForService($name); $container = $container ?? \OC::$server; @@ -139,17 +101,13 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { return false; } - public function restoreAllServices() { - if (!empty($this->services)) { - if (!empty($this->services)) { - foreach ($this->services as $name => $service) { - $this->restoreService($name); - } - } + public function restoreAllServices(): void { + foreach ($this->services as $name => $service) { + $this->restoreService($name); } } - protected function getTestTraits() { + protected function getTestTraits(): array { $traits = []; $class = $this; do { @@ -177,6 +135,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { if (is_null(self::$realDatabase)) { self::$realDatabase = Server::get(IDBConnection::class); } + /** @psalm-suppress InternalMethod */ \OC::$server->registerService(IDBConnection::class, function (): void { $this->fail('Your test case is not allowed to access the database.'); }); @@ -196,6 +155,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { // restore database connection if (!$this->IsDatabaseAccessAllowed()) { + /** @psalm-suppress InternalMethod */ \OC::$server->registerService(IDBConnection::class, function () { return self::$realDatabase; }); @@ -337,9 +297,13 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { self::tearDownAfterClassCleanStrayLocks(); // Ensure we start with fresh instances of some classes to reduce side-effects between tests + /** @psalm-suppress DeprecatedMethod */ unset(\OC::$server[Factory::class]); + /** @psalm-suppress DeprecatedMethod */ unset(\OC::$server[AppFetcher::class]); + /** @psalm-suppress DeprecatedMethod */ unset(\OC::$server[Installer::class]); + /** @psalm-suppress DeprecatedMethod */ unset(\OC::$server[Updater::class]); /** @var SetupManager $setupManager */ @@ -364,30 +328,24 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { /** * Remove all entries from the share table - * - * @param IQueryBuilder $queryBuilder */ - protected static function tearDownAfterClassCleanShares(IQueryBuilder $queryBuilder) { + protected static function tearDownAfterClassCleanShares(IQueryBuilder $queryBuilder): void { $queryBuilder->delete('share') ->executeStatement(); } /** * Remove all entries from the storages table - * - * @param IQueryBuilder $queryBuilder */ - protected static function tearDownAfterClassCleanStorages(IQueryBuilder $queryBuilder) { + protected static function tearDownAfterClassCleanStorages(IQueryBuilder $queryBuilder): void { $queryBuilder->delete('storages') ->executeStatement(); } /** * Remove all entries from the filecache table - * - * @param IQueryBuilder $queryBuilder */ - protected static function tearDownAfterClassCleanFileCache(IQueryBuilder $queryBuilder) { + protected static function tearDownAfterClassCleanFileCache(IQueryBuilder $queryBuilder): void { $queryBuilder->delete('filecache') ->runAcrossAllShards() ->executeStatement(); @@ -398,7 +356,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { * * @param string $dataDir */ - protected static function tearDownAfterClassCleanStrayDataFiles($dataDir) { + protected static function tearDownAfterClassCleanStrayDataFiles(string $dataDir): void { $knownEntries = [ 'nextcloud.log' => true, 'audit.log' => true, @@ -423,7 +381,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { * * @param string $dir */ - protected static function tearDownAfterClassCleanStrayDataUnlinkDir($dir) { + protected static function tearDownAfterClassCleanStrayDataUnlinkDir(string $dir): void { if ($dh = @opendir($dir)) { while (($file = readdir($dh)) !== false) { if (Filesystem::isIgnoredDir($file)) { @@ -444,14 +402,14 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { /** * Clean up the list of hooks */ - protected static function tearDownAfterClassCleanStrayHooks() { + protected static function tearDownAfterClassCleanStrayHooks(): void { \OC_Hook::clear(); } /** * Clean up the list of locks */ - protected static function tearDownAfterClassCleanStrayLocks() { + protected static function tearDownAfterClassCleanStrayLocks(): void { Server::get(ILockingProvider::class)->releaseAll(); } @@ -461,48 +419,46 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { * * @param string $user user id or empty for a generic FS */ - protected static function loginAsUser($user = '') { + protected static function loginAsUser(string $user = ''): void { self::logout(); Filesystem::tearDown(); \OC_User::setUserId($user); - $userObject = Server::get(IUserManager::class)->get($user); + $userManager = Server::get(IUserManager::class); + $setupManager = Server::get(SetupManager::class); + $userObject = $userManager->get($user); if (!is_null($userObject)) { $userObject->updateLastLoginTimestamp(); - } - \OC_Util::setupFS($user); - if (Server::get(IUserManager::class)->userExists($user)) { - \OC::$server->getUserFolder($user); + $setupManager->setupForUser($userObject); + $rootFolder = Server::get(IRootFolder::class); + $rootFolder->getUserFolder($user); } } /** * Logout the current user and tear down the filesystem. */ - protected static function logout() { - \OC_Util::tearDownFS(); - \OC_User::setUserId(''); + protected static function logout(): void { + Server::get(SetupManager::class)->tearDown(); + $userSession = Server::get(\OC\User\Session::class); + $userSession->getSession()->set('user_id', ''); // needed for fully logout - Server::get(IUserSession::class)->setUser(null); + $userSession->setUser(null); } /** * Run all commands pushed to the bus */ - protected function runCommands() { - // get the user for which the fs is setup - $view = Filesystem::getView(); - if ($view) { - [, $user] = explode('/', $view->getRoot()); - } else { - $user = null; - } + protected function runCommands(): void { + $setupManager = Server::get(SetupManager::class); + $session = Server::get(IUserSession::class); + $user = $session->getUser(); - \OC_Util::tearDownFS(); // command can't reply on the fs being setup + $setupManager->tearDown(); // commands can't reply on the fs being setup $this->commandBus->run(); - \OC_Util::tearDownFS(); + $setupManager->tearDown(); if ($user) { - \OC_Util::setupFS($user); + $setupManager->setupForUser($user); } } @@ -518,7 +474,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { * @return boolean true if the file is locked with the * given type, false otherwise */ - protected function isFileLocked($view, $path, $type, $onMountPoint = false) { + protected function isFileLocked(View $view, string $path, int $type, bool $onMountPoint = false) { // Note: this seems convoluted but is necessary because // the format of the lock key depends on the storage implementation // (in our case mostly md5) @@ -543,6 +499,9 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { } } + /** + * @return list + */ protected function getGroupAnnotations(): array { if (method_exists($this, 'getAnnotations')) { $annotations = $this->getAnnotations(); @@ -553,7 +512,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { $doc = $r->getDocComment(); if (class_exists(Group::class)) { - $attributes = array_map(function (\ReflectionAttribute $attribute) { + $attributes = array_map(function (\ReflectionAttribute $attribute): string { /** @var Group $group */ $group = $attribute->newInstance(); return $group->name(); @@ -568,75 +527,6 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { protected function IsDatabaseAccessAllowed(): bool { $annotations = $this->getGroupAnnotations(); - if (isset($annotations)) { - if (in_array('DB', $annotations) || in_array('SLOWDB', $annotations)) { - return true; - } - } - - return false; - } - - /** - * @param string $expectedHtml - * @param string $template - * @param array $vars - */ - protected function assertTemplate($expectedHtml, $template, $vars = []) { - $requestToken = 12345; - /** @var Defaults|\PHPUnit\Framework\MockObject\MockObject $l10n */ - $theme = $this->getMockBuilder('\OCP\Defaults') - ->disableOriginalConstructor()->getMock(); - $theme->expects($this->any()) - ->method('getName') - ->willReturn('Nextcloud'); - /** @var IL10N|\PHPUnit\Framework\MockObject\MockObject $l10n */ - $l10n = $this->getMockBuilder(IL10N::class) - ->disableOriginalConstructor()->getMock(); - $l10n - ->expects($this->any()) - ->method('t') - ->willReturnCallback(function ($text, $parameters = []) { - return vsprintf($text, $parameters); - }); - - $t = new Base($template, $requestToken, $l10n, $theme); - $buf = $t->fetchPage($vars); - $this->assertHtmlStringEqualsHtmlString($expectedHtml, $buf); - } - - /** - * @param string $expectedHtml - * @param string $actualHtml - * @param string $message - */ - protected function assertHtmlStringEqualsHtmlString($expectedHtml, $actualHtml, $message = '') { - $expected = new DOMDocument(); - $expected->preserveWhiteSpace = false; - $expected->formatOutput = true; - $expected->loadHTML($expectedHtml); - - $actual = new DOMDocument(); - $actual->preserveWhiteSpace = false; - $actual->formatOutput = true; - $actual->loadHTML($actualHtml); - $this->removeWhitespaces($actual); - - $expectedHtml1 = $expected->saveHTML(); - $actualHtml1 = $actual->saveHTML(); - self::assertEquals($expectedHtml1, $actualHtml1, $message); - } - - - private function removeWhitespaces(DOMNode $domNode) { - foreach ($domNode->childNodes as $node) { - if ($node->hasChildNodes()) { - $this->removeWhitespaces($node); - } else { - if ($node instanceof \DOMText && $node->isWhitespaceInElementContent()) { - $domNode->removeChild($node); - } - } - } + return in_array('DB', $annotations) || in_array('SLOWDB', $annotations); } }