Skip to content

Commit df2f3a8

Browse files
CarlSchwanCarlSchwan
committed
refactor(integration): Refactor and fix federation integration tests
Signed-off-by: Carl Schwan <[email protected]>
1 parent 3183ea7 commit df2f3a8

File tree

14 files changed

+110
-137
lines changed

14 files changed

+110
-137
lines changed

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -63,24 +63,19 @@ public function __construct(
6363
) {
6464
}
6565

66-
/**
67-
* Return the identifier of this provider.
68-
*
69-
* @return string Containing only [a-zA-Z0-9]
70-
*/
71-
public function identifier() {
66+
#[Override]
67+
public function identifier(): string {
7268
return 'ocFederatedSharing';
7369
}
7470

7571
/**
7672
* Share a path
7773
*
78-
* @param IShare $share
79-
* @return IShare The share object
8074
* @throws ShareNotFound
8175
* @throws \Exception
8276
*/
83-
public function create(IShare $share) {
77+
#[Override]
78+
public function create(IShare $share): IShare {
8479
$shareWith = $share->getSharedWith();
8580
$itemSource = $share->getNodeId();
8681
$itemType = $share->getNodeType();
@@ -141,7 +136,7 @@ public function create(IShare $share) {
141136
if ($remoteShare) {
142137
try {
143138
$ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']);
144-
$shareId = (string)$this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate);
139+
$shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate);
145140
$share->setId($shareId);
146141
[$token, $remoteId] = $this->askOwnerToReShare($shareWith, $share, $shareId);
147142
// remote share was create successfully if we get a valid token as return
@@ -176,7 +171,7 @@ public function create(IShare $share) {
176171
*/
177172
protected function createFederatedShare(IShare $share): string {
178173
$token = $this->tokenHandler->generateToken();
179-
$shareId = (string)$this->addShareToDB(
174+
$shareId = $this->addShareToDB(
180175
$share->getNodeId(),
181176
$share->getNodeType(),
182177
$share->getSharedWith(),
@@ -292,9 +287,8 @@ protected function getShareFromExternalShareTable(IShare $share) {
292287
* @param string $token
293288
* @param int $shareType
294289
* @param \DateTime $expirationDate
295-
* @return int
296290
*/
297-
private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType, $expirationDate) {
291+
private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $shareType, $expirationDate): string {
298292
$qb = $this->dbConnection->getQueryBuilder();
299293
$qb->insert('share')
300294
->setValue('share_type', $qb->createNamedParameter($shareType))
@@ -316,7 +310,7 @@ private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ui
316310
$qb->setValue('file_target', $qb->createNamedParameter(''));
317311

318312
$qb->executeStatement();
319-
return $qb->getLastInsertId();
313+
return (string)$qb->getLastInsertId();
320314
}
321315

322316
/**

‎apps/files_external/tests/Config/UserPlaceholderHandlerTest.php‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
use OCP\IUserSession;
1515
use OCP\Share\Exceptions\ShareNotFound;
1616
use OCP\Share\IManager;
17+
use PHPUnit\Framework\Attributes\DataProvider;
1718
use PHPUnit\Framework\MockObject\MockObject;
19+
use Test\TestCase;
1820

19-
class UserPlaceholderHandlerTest extends \Test\TestCase {
21+
class UserPlaceholderHandlerTest extends TestCase {
2022
protected IUser&MockObject $user;
2123
protected IUserSession&MockObject $session;
2224
protected IManager&MockObject $shareManager;
@@ -34,6 +36,9 @@ protected function setUp(): void {
3436
$this->session = $this->createMock(IUserSession::class);
3537
$this->shareManager = $this->createMock(IManager::class);
3638
$this->request = $this->createMock(IRequest::class);
39+
$this->request->method('getParam')
40+
->with('token')
41+
->willReturn('foo');
3742
$this->userManager = $this->createMock(IUserManager::class);
3843

3944
$this->handler = new UserPlaceholderHandler($this->session, $this->shareManager, $this->request, $this->userManager);
@@ -53,16 +58,17 @@ public static function optionProvider(): array {
5358
];
5459
}
5560

56-
#[\PHPUnit\Framework\Attributes\DataProvider('optionProvider')]
61+
#[DataProvider('optionProvider')]
5762
public function testHandle(string|array $option, string|array $expected): void {
5863
$this->setUser();
5964
$this->assertSame($expected, $this->handler->handle($option));
6065
}
6166

62-
#[\PHPUnit\Framework\Attributes\DataProvider('optionProvider')]
67+
#[DataProvider('optionProvider')]
6368
public function testHandleNoUser(string|array $option): void {
6469
$this->shareManager->expects($this->once())
6570
->method('getShareByToken')
71+
->with('foo')
6672
->willThrowException(new ShareNotFound());
6773
$this->assertSame($option, $this->handler->handle($option));
6874
}

‎apps/files_sharing/lib/Command/CleanupRemoteStorages.php‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
namespace OCA\Files_Sharing\Command;
99

10+
use OCA\Files_Sharing\External\ExternalShareMapper;
1011
use OCP\DB\QueryBuilder\IQueryBuilder;
1112
use OCP\Federation\ICloudIdManager;
1213
use OCP\IDBConnection;
@@ -22,13 +23,13 @@
2223
class CleanupRemoteStorages extends Command {
2324

2425
public function __construct(
25-
protected IDBConnection $connection,
26-
private ICloudIdManager $cloudIdManager,
26+
protected readonly IDBConnection $connection,
27+
private readonly ICloudIdManager $cloudIdManager,
2728
) {
2829
parent::__construct();
2930
}
3031

31-
protected function configure() {
32+
protected function configure(): void {
3233
$this
3334
->setName('sharing:cleanup-remote-storages')
3435
->setDescription('Cleanup shared storage entries that have no matching entry in the shares_external table')
@@ -74,10 +75,11 @@ public function execute(InputInterface $input, OutputInterface $output): int {
7475
}
7576
}
7677
}
77-
return 0;
78+
79+
return Command::SUCCESS;
7880
}
7981

80-
public function countFiles($numericId, OutputInterface $output) {
82+
public function countFiles($numericId, OutputInterface $output): void {
8183
$queryBuilder = $this->connection->getQueryBuilder();
8284
$queryBuilder->select($queryBuilder->func()->count('fileid'))
8385
->from('filecache')
@@ -92,7 +94,7 @@ public function countFiles($numericId, OutputInterface $output) {
9294
$output->writeln("$count files can be deleted for storage $numericId");
9395
}
9496

95-
public function deleteStorage($id, $numericId, OutputInterface $output) {
97+
public function deleteStorage($id, $numericId, OutputInterface $output): void {
9698
$queryBuilder = $this->connection->getQueryBuilder();
9799
$queryBuilder->delete('storages')
98100
->where($queryBuilder->expr()->eq(
@@ -106,7 +108,7 @@ public function deleteStorage($id, $numericId, OutputInterface $output) {
106108
$this->deleteFiles($numericId, $output);
107109
}
108110

109-
public function deleteFiles($numericId, OutputInterface $output) {
111+
public function deleteFiles($numericId, OutputInterface $output): void {
110112
$queryBuilder = $this->connection->getQueryBuilder();
111113
$queryBuilder->delete('filecache')
112114
->where($queryBuilder->expr()->eq(
@@ -119,7 +121,11 @@ public function deleteFiles($numericId, OutputInterface $output) {
119121
$output->writeln("deleted $count files");
120122
}
121123

122-
public function getRemoteStorages() {
124+
/**
125+
* @return array<string, int>
126+
* @throws \OCP\DB\Exception
127+
*/
128+
private function getRemoteStorages(): array {
123129
$queryBuilder = $this->connection->getQueryBuilder();
124130
$queryBuilder->select(['id', 'numeric_id'])
125131
->from('storages')
@@ -148,7 +154,10 @@ public function getRemoteStorages() {
148154
return $remoteStorages;
149155
}
150156

151-
public function getRemoteShareIds() {
157+
/**
158+
* @return array<string, string>
159+
*/
160+
private function getRemoteShareIds(): array {
152161
$queryBuilder = $this->connection->getQueryBuilder();
153162
$queryBuilder->select(['id', 'share_token', 'owner', 'remote'])
154163
->from('share_external');
@@ -159,7 +168,6 @@ public function getRemoteShareIds() {
159168
while ($row = $result->fetchAssociative()) {
160169
$cloudId = $this->cloudIdManager->getCloudId($row['owner'], $row['remote']);
161170
$remote = $cloudId->getRemote();
162-
163171
$remoteShareIds[$row['id']] = 'shared::' . md5($row['share_token'] . '@' . $remote);
164172
}
165173
$result->closeCursor();

‎apps/files_sharing/lib/Controller/RemoteController.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
use OCP\AppFramework\OCS\OCSNotFoundException;
1818
use OCP\AppFramework\OCSController;
1919
use OCP\Files\IRootFolder;
20-
use OCP\Files\NotFoundException;
21-
use OCP\Files\NotPermittedException;
2220
use OCP\IRequest;
2321
use Psr\Log\LoggerInterface;
2422

@@ -114,7 +112,7 @@ private function extendShareInfo(ExternalShare $share): array {
114112

115113
try {
116114
$mountPointNode = $userFolder->get($share->getMountpoint());
117-
} catch (NotPermittedException|NotFoundException) {
115+
} catch (\Exception) {
118116
return $share->jsonSerialize();
119117
}
120118

@@ -182,7 +180,7 @@ public function unshare(string $id): DataResponse {
182180
throw new OCSNotFoundException('Share does not exist');
183181
}
184182

185-
$mountPoint = '/' . $this->userId . '/files' . $shareInfo->getMountPoint();
183+
$mountPoint = '/' . $this->userId . '/files' . $shareInfo->getMountpoint();
186184

187185
if ($this->externalManager->removeShare($mountPoint) === true) {
188186
return new DataResponse();

‎apps/files_sharing/lib/External/ExternalShare.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@
3131
* @method void setRemoteId(string $remoteId)
3232
* @method string getShareToken()
3333
* @method void setShareToken(string $shareToken)
34-
* @method string getPassword()
35-
* @method void setPassword(string $password)
34+
* @method string|null getPassword()
35+
* @method void setPassword(?string $password)
3636
* @method string getName()
3737
* @method string getOwner()
3838
* @method void setOwner(string $owner)
@@ -97,7 +97,7 @@ public function jsonSerialize(): array {
9797
$parent = $this->getParent();
9898
return [
9999
'id' => $this->getId(),
100-
'parent' => $parent === '-1' ? null : $parent,
100+
'parent' => $parent,
101101
'share_type' => $this->getShareType() ?? IShare::TYPE_USER, // unfortunately nullable on the DB level, but never null.
102102
'remote' => $this->getRemote(),
103103
'remote_id' => $this->getRemoteId(),
@@ -106,7 +106,7 @@ public function jsonSerialize(): array {
106106
'owner' => $this->getOwner(),
107107
'user' => $this->getUser(),
108108
'mountpoint' => $this->getMountpoint(),
109-
'accepted' => $this->getAccepted() === IShare::STATUS_ACCEPTED,
109+
'accepted' => $this->getAccepted(),
110110

111111
// Added later on
112112
'file_id' => null,

‎apps/files_sharing/lib/External/ExternalShareMapper.php‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,6 @@ public function getUserShare(ExternalShare $parentShare, IUser $user): ExternalS
7575
return $this->findEntity($qb);
7676
}
7777

78-
public function get() {
79-
80-
}
81-
8278
public function getByMountPointAndUser(string $mountPoint, IUser $user) {
8379
$qb = $this->db->getQueryBuilder();
8480
$qb->select('*')
@@ -151,6 +147,17 @@ public function deleteGroupShares(IGroup $group): void {
151147
}
152148
}
153149

150+
/**
151+
* @return \Generator<ExternalShare>
152+
*/
153+
public function getAllShares(): \Generator {
154+
$qb = $this->db->getQueryBuilder();
155+
$qb->select('*')
156+
->from(self::TABLE_NAME);
157+
158+
return $this->yieldEntities($qb);
159+
}
160+
154161
/**
155162
* Return a list of shares for the user.
156163
*
@@ -201,4 +208,10 @@ public function getShares(IUser $user, ?int $status): array {
201208
}
202209
return array_values($shares);
203210
}
211+
212+
public function deleteAll(): void {
213+
$qb = $this->db->getQueryBuilder();
214+
$qb->delete('share_external')
215+
->executeStatement();
216+
}
204217
}

‎apps/files_sharing/lib/ResponseDefinitions.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,15 @@
8181
* }
8282
*
8383
* @psalm-type Files_SharingRemoteShare = array{
84-
* accepted: bool,
84+
* accepted: int,
8585
* file_id: int|null,
8686
* id: string,
8787
* mimetype: string|null,
8888
* mountpoint: string,
8989
* mtime: int|null,
9090
* name: string,
9191
* owner: string,
92-
* parent: string|null,
92+
* parent: string,
9393
* permissions: int|null,
9494
* remote: string,
9595
* remote_id: string,

‎apps/files_sharing/tests/CapabilitiesTest.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,9 @@
1919
use OCP\IConfig;
2020
use OCP\IDateTimeZone;
2121
use OCP\IGroupManager;
22-
use OCP\IURLGenerator;
2322
use OCP\IUserManager;
2423
use OCP\IUserSession;
2524
use OCP\L10N\IFactory;
26-
use OCP\Mail\IMailer;
2725
use OCP\Security\IHasher;
2826
use OCP\Security\ISecureRandom;
2927
use OCP\Share\IProviderFactory;

‎apps/files_sharing/tests/Command/CleanupRemoteStoragesTest.php‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,17 @@
1515
use OCP\IDBConnection;
1616
use OCP\Server;
1717
use OCP\Snowflake\IGenerator;
18+
use PHPUnit\Framework\Attributes\Group;
1819
use PHPUnit\Framework\MockObject\MockObject;
1920
use Symfony\Component\Console\Input\InputInterface;
2021
use Symfony\Component\Console\Output\OutputInterface;
2122
use Test\TestCase;
2223

23-
/**
24-
* Class CleanupRemoteStoragesTest
25-
*
26-
*
27-
* @package OCA\Files_Sharing\Tests\Command
28-
*/
29-
#[\PHPUnit\Framework\Attributes\Group('DB')]
24+
#[Group('DB')]
3025
class CleanupRemoteStoragesTest extends TestCase {
3126

3227
protected IDBConnection $connection;
28+
protected ExternalShareMapper $mapper;
3329
protected CleanupRemoteStorages $command;
3430
private ICloudIdManager&MockObject $cloudIdManager;
3531

@@ -47,6 +43,7 @@ protected function setUp(): void {
4743
parent::setUp();
4844

4945
$this->connection = Server::get(IDBConnection::class);
46+
$this->mapper = Server::get(ExternalShareMapper::class);
5047

5148
$storageQuery = Server::get(IDBConnection::class)->getQueryBuilder();
5249
$storageQuery->insert('storages')
@@ -74,7 +71,7 @@ protected function setUp(): void {
7471
$externalShare->setOwner('irrelevant');
7572
$externalShare->setUser($storage['user']);
7673
$externalShare->setMountpoint('irrelevant');
77-
Server::get(ExternalShareMapper::class)->insert($externalShare);
74+
$this->mapper->insert($externalShare);
7875
}
7976

8077
if (isset($storage['files_count'])) {

‎build/integration/features/bootstrap/AppConfiguration.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
use Behat\Behat\Hook\Scope\AfterScenarioScope;
99
use Behat\Behat\Hook\Scope\BeforeScenarioScope;
10+
use Behat\Gherkin\Node\TableNode;
1011
use PHPUnit\Framework\Assert;
1112
use Psr\Http\Message\ResponseInterface;
1213

@@ -19,8 +20,8 @@ trait AppConfiguration {
1920
/** @var ResponseInterface */
2021
private $response = null;
2122

22-
abstract public function sendingTo($verb, $url);
23-
abstract public function sendingToWith($verb, $url, $body);
23+
abstract public function sendingTo(string $verb, string $url);
24+
abstract public function sendingToWith(string $verb, string $url, ?TableNode $body);
2425
abstract public function theOCSStatusCodeShouldBe($statusCode);
2526
abstract public function theHTTPStatusCodeShouldBe($statusCode);
2627

0 commit comments

Comments
 (0)