Skip to content

Commit f2d3b2e

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

File tree

16 files changed

+124
-140
lines changed

16 files changed

+124
-140
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: 26 additions & 11 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,14 @@
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,
28+
private readonly ExternalShareMapper $externalShareMapper,
2729
) {
2830
parent::__construct();
2931
}
3032

31-
protected function configure() {
33+
protected function configure(): void {
3234
$this
3335
->setName('sharing:cleanup-remote-storages')
3436
->setDescription('Cleanup shared storage entries that have no matching entry in the shares_external table')
@@ -37,6 +39,12 @@ protected function configure() {
3739
null,
3840
InputOption::VALUE_NONE,
3941
'only show which storages would be deleted'
42+
)
43+
->addOption(
44+
'all',
45+
null,
46+
InputOption::VALUE_NONE,
47+
'Delete every external shares and their storage',
4048
);
4149
}
4250

@@ -46,11 +54,12 @@ public function execute(InputInterface $input, OutputInterface $output): int {
4654
$output->writeln(count($remoteStorages) . ' remote storage(s) need(s) to be checked');
4755

4856
$remoteShareIds = $this->getRemoteShareIds();
57+
$all = $input->getOption('all');
4958

5059
$output->writeln(count($remoteShareIds) . ' remote share(s) exist');
5160

5261
foreach ($remoteShareIds as $id => $remoteShareId) {
53-
if (isset($remoteStorages[$remoteShareId])) {
62+
if ($all || isset($remoteStorages[$remoteShareId])) {
5463
if ($input->getOption('dry-run') || $output->isVerbose()) {
5564
$output->writeln("<info>$remoteShareId belongs to remote share $id</info>");
5665
}
@@ -74,10 +83,14 @@ public function execute(InputInterface $input, OutputInterface $output): int {
7483
}
7584
}
7685
}
77-
return 0;
86+
87+
if ($all) {
88+
$this->externalShareMapper->deleteAll();
89+
}
90+
return Command::SUCCESS;
7891
}
7992

80-
public function countFiles($numericId, OutputInterface $output) {
93+
public function countFiles($numericId, OutputInterface $output): void {
8194
$queryBuilder = $this->connection->getQueryBuilder();
8295
$queryBuilder->select($queryBuilder->func()->count('fileid'))
8396
->from('filecache')
@@ -92,7 +105,7 @@ public function countFiles($numericId, OutputInterface $output) {
92105
$output->writeln("$count files can be deleted for storage $numericId");
93106
}
94107

95-
public function deleteStorage($id, $numericId, OutputInterface $output) {
108+
public function deleteStorage($id, $numericId, OutputInterface $output): void {
96109
$queryBuilder = $this->connection->getQueryBuilder();
97110
$queryBuilder->delete('storages')
98111
->where($queryBuilder->expr()->eq(
@@ -106,7 +119,7 @@ public function deleteStorage($id, $numericId, OutputInterface $output) {
106119
$this->deleteFiles($numericId, $output);
107120
}
108121

109-
public function deleteFiles($numericId, OutputInterface $output) {
122+
public function deleteFiles($numericId, OutputInterface $output): void {
110123
$queryBuilder = $this->connection->getQueryBuilder();
111124
$queryBuilder->delete('filecache')
112125
->where($queryBuilder->expr()->eq(
@@ -119,7 +132,7 @@ public function deleteFiles($numericId, OutputInterface $output) {
119132
$output->writeln("deleted $count files");
120133
}
121134

122-
public function getRemoteStorages() {
135+
private function getRemoteStorages(): array {
123136
$queryBuilder = $this->connection->getQueryBuilder();
124137
$queryBuilder->select(['id', 'numeric_id'])
125138
->from('storages')
@@ -148,7 +161,10 @@ public function getRemoteStorages() {
148161
return $remoteStorages;
149162
}
150163

151-
public function getRemoteShareIds() {
164+
/**
165+
* @return array<string, string>
166+
*/
167+
private function getRemoteShareIds(): array {
152168
$queryBuilder = $this->connection->getQueryBuilder();
153169
$queryBuilder->select(['id', 'share_token', 'owner', 'remote'])
154170
->from('share_external');
@@ -159,7 +175,6 @@ public function getRemoteShareIds() {
159175
while ($row = $result->fetchAssociative()) {
160176
$cloudId = $this->cloudIdManager->getCloudId($row['owner'], $row['remote']);
161177
$remote = $cloudId->getRemote();
162-
163178
$remoteShareIds[$row['id']] = 'shared::' . md5($row['share_token'] . '@' . $remote);
164179
}
165180
$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;

0 commit comments

Comments
 (0)