Skip to content

Commit 98b5b77

Browse files
CarlSchwanCarlSchwan
committed
refactor(IShare): Add typing for node
This might also improve a bit the performance. Signed-off-by: Carl Schwan <carlschwan@kde.org>
1 parent 8f8b441 commit 98b5b77

File tree

13 files changed

+173
-238
lines changed

13 files changed

+173
-238
lines changed

‎apps/federatedfilesharing/tests/Controller/MountPublicLinkControllerTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ protected function setUp(): void {
5858
$this->rootFolder = $this->createMock(IRootFolder::class);
5959
$this->userManager = $this->createMock(IUserManager::class);
6060
$this->share = new Share($this->rootFolder, $this->userManager);
61+
$this->share->setId('42');
6162
$this->session = $this->createMock(ISession::class);
6263
$this->l10n = $this->createMock(IL10N::class);
6364
$this->userSession = $this->createMock(IUserSession::class);

‎apps/federatedfilesharing/tests/FederatedShareProviderTest.php‎

Lines changed: 19 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99
namespace OCA\FederatedFileSharing\Tests;
1010

11+
use LogicException;
1112
use OC\Federation\CloudIdManager;
1213
use OCA\FederatedFileSharing\AddressHandler;
1314
use OCA\FederatedFileSharing\FederatedShareProvider;
@@ -250,16 +251,12 @@ public function testCreateCouldNotFindServer(): void {
250251
$this->assertEquals('Sharing myFile failed, could not find user@server.com, maybe the server is currently unreachable or uses a self-signed certificate.', $e->getMessage());
251252
}
252253

253-
$qb = $this->connection->getQueryBuilder();
254-
$stmt = $qb->select('*')
255-
->from('share')
256-
->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId())))
257-
->executeQuery();
258-
259-
$data = $stmt->fetchAssociative();
260-
$stmt->closeCursor();
261-
262-
$this->assertFalse($data);
254+
try {
255+
$share->getId();
256+
$this->fail();
257+
} catch (LogicException $e) {
258+
// object is expected to not have been created
259+
}
263260
}
264261

265262
public function testCreateException(): void {
@@ -311,16 +308,12 @@ public function testCreateException(): void {
311308
$this->assertEquals('Sharing myFile failed, could not find user@server.com, maybe the server is currently unreachable or uses a self-signed certificate.', $e->getMessage());
312309
}
313310

314-
$qb = $this->connection->getQueryBuilder();
315-
$stmt = $qb->select('*')
316-
->from('share')
317-
->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId())))
318-
->executeQuery();
319-
320-
$data = $stmt->fetchAssociative();
321-
$stmt->closeCursor();
322-
323-
$this->assertFalse($data);
311+
try {
312+
$share->getId();
313+
$this->fail();
314+
} catch (LogicException $e) {
315+
// object is expected to not have been created
316+
}
324317
}
325318

326319
public function testCreateShareWithSelf(): void {
@@ -354,16 +347,12 @@ public function testCreateShareWithSelf(): void {
354347
$this->assertEquals('Not allowed to create a federated share to the same account', $e->getMessage());
355348
}
356349

357-
$qb = $this->connection->getQueryBuilder();
358-
$stmt = $qb->select('*')
359-
->from('share')
360-
->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId())))
361-
->executeQuery();
362-
363-
$data = $stmt->fetchAssociative();
364-
$stmt->closeCursor();
365-
366-
$this->assertFalse($data);
350+
try {
351+
$share->getId();
352+
$this->fail();
353+
} catch (LogicException $e) {
354+
// object is expected to not have been created
355+
}
367356
}
368357

369358
public function testCreateAlreadyShared(): void {

‎apps/files_sharing/tests/Controller/ShareAPIControllerTest.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ public function testGetGetShareNotExists() {
592592
*/
593593

594594
public function createShare(
595-
int $id,
595+
string $id,
596596
int $shareType,
597597
?string $sharedWith,
598598
string $sharedBy,
@@ -658,7 +658,7 @@ public static function dataGetShare(): array {
658658

659659
// File shared with user
660660
$share = [
661-
100,
661+
'100',
662662
IShare::TYPE_USER,
663663
'userId',
664664
'initiatorId',
@@ -677,7 +677,7 @@ public static function dataGetShare(): array {
677677
[],
678678
];
679679
$expected = [
680-
'id' => 100,
680+
'id' => '100',
681681
'share_type' => IShare::TYPE_USER,
682682
'share_with' => 'userId',
683683
'share_with_displayname' => 'userDisplay',
@@ -717,7 +717,7 @@ public static function dataGetShare(): array {
717717

718718
// Folder shared with group
719719
$share = [
720-
101,
720+
'101',
721721
IShare::TYPE_GROUP,
722722
'groupId',
723723
'initiatorId',
@@ -736,7 +736,7 @@ public static function dataGetShare(): array {
736736
[],
737737
];
738738
$expected = [
739-
'id' => 101,
739+
'id' => '101',
740740
'share_type' => IShare::TYPE_GROUP,
741741
'share_with' => 'groupId',
742742
'share_with_displayname' => 'groupId',
@@ -776,7 +776,7 @@ public static function dataGetShare(): array {
776776
// File shared by link with Expire
777777
$expire = \DateTime::createFromFormat('Y-m-d h:i:s', '2000-01-02 01:02:03');
778778
$share = [
779-
101,
779+
'101',
780780
IShare::TYPE_LINK,
781781
null,
782782
'initiatorId',
@@ -794,7 +794,7 @@ public static function dataGetShare(): array {
794794
'first link share'
795795
];
796796
$expected = [
797-
'id' => 101,
797+
'id' => '101',
798798
'share_type' => IShare::TYPE_LINK,
799799
'password' => 'password',
800800
'share_with' => 'password',

‎apps/files_sharing/tests/MountProviderTest.php‎

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ class MountProviderTest extends \Test\TestCase {
3939
protected function setUp(): void {
4040
parent::setUp();
4141

42-
$this->config = $this->getMockBuilder(IConfig::class)->getMock();
43-
$this->user = $this->getMockBuilder(IUser::class)->getMock();
44-
$this->loader = $this->getMockBuilder('OCP\Files\Storage\IStorageFactory')->getMock();
45-
$this->shareManager = $this->getMockBuilder(IManager::class)->getMock();
46-
$this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
42+
$this->config = $this->createMock(IConfig::class);
43+
$this->user = $this->createMock(IUser::class);
44+
$this->loader = $this->createMock(IStorageFactory::class);
45+
$this->shareManager = $this->createMock(IManager::class);
46+
$this->logger = $this->createMock(LoggerInterface::class);
4747
$eventDispatcher = $this->createMock(IEventDispatcher::class);
4848
$cacheFactory = $this->createMock(ICacheFactory::class);
4949
$cacheFactory->method('createLocal')
@@ -53,11 +53,7 @@ protected function setUp(): void {
5353
$this->provider = new MountProvider($this->config, $this->shareManager, $this->logger, $eventDispatcher, $cacheFactory, $mountManager);
5454
}
5555

56-
private function makeMockShareAttributes($attrs) {
57-
if ($attrs === null) {
58-
return null;
59-
}
60-
56+
private function makeMockShareAttributes(array $attrs): IShareAttributes&MockObject {
6157
$shareAttributes = $this->createMock(IShareAttributes::class);
6258
$shareAttributes->method('toArray')->willReturn($attrs);
6359
$shareAttributes->method('getAttribute')->willReturnCallback(
@@ -74,14 +70,14 @@ function ($scope, $key) use ($attrs) {
7470
return $shareAttributes;
7571
}
7672

77-
private function makeMockShare($id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $attributes = null) {
73+
private function makeMockShare(string $id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $attributes = null) {
7874
$share = $this->createMock(IShare::class);
7975
$share->expects($this->any())
8076
->method('getPermissions')
8177
->willReturn($permissions);
8278
$share->expects($this->any())
8379
->method('getAttributes')
84-
->willReturn($this->makeMockShareAttributes($attributes));
80+
->willReturn($attributes === null ? null : $this->makeMockShareAttributes($attributes));
8581
$share->expects($this->any())
8682
->method('getShareOwner')
8783
->willReturn($owner);
@@ -114,25 +110,25 @@ public function testExcludeShares(): void {
114110
$attr1 = [];
115111
$attr2 = [['scope' => 'permission', 'key' => 'download', 'value' => true]];
116112
$userShares = [
117-
$this->makeMockShare(1, 100, 'user2', '/share2', 0, $attr1),
118-
$this->makeMockShare(2, 100, 'user2', '/share2', 31, $attr2),
113+
$this->makeMockShare('1', 100, 'user2', '/share2', 0, $attr1),
114+
$this->makeMockShare('2', 100, 'user2', '/share2', 31, $attr2),
119115
];
120116
$groupShares = [
121-
$this->makeMockShare(3, 100, 'user2', '/share2', 0, $attr1),
122-
$this->makeMockShare(4, 101, 'user2', '/share4', 31, $attr2),
123-
$this->makeMockShare(5, 100, 'user1', '/share4', 31, $attr2),
117+
$this->makeMockShare('3', 100, 'user2', '/share2', 0, $attr1),
118+
$this->makeMockShare('4', 101, 'user2', '/share4', 31, $attr2),
119+
$this->makeMockShare('5', 100, 'user1', '/share4', 31, $attr2),
124120
];
125121
$roomShares = [
126-
$this->makeMockShare(6, 102, 'user2', '/share6', 0),
127-
$this->makeMockShare(7, 102, 'user1', '/share6', 31),
128-
$this->makeMockShare(8, 102, 'user2', '/share6', 31),
129-
$this->makeMockShare(9, 102, 'user2', '/share6', 31),
122+
$this->makeMockShare('6', 102, 'user2', '/share6', 0),
123+
$this->makeMockShare('7', 102, 'user1', '/share6', 31),
124+
$this->makeMockShare('8', 102, 'user2', '/share6', 31),
125+
$this->makeMockShare('9', 102, 'user2', '/share6', 31),
130126
];
131127
$deckShares = [
132-
$this->makeMockShare(10, 103, 'user2', '/share7', 0),
133-
$this->makeMockShare(11, 103, 'user1', '/share7', 31),
134-
$this->makeMockShare(12, 103, 'user2', '/share7', 31),
135-
$this->makeMockShare(13, 103, 'user2', '/share7', 31),
128+
$this->makeMockShare('10', 103, 'user2', '/share7', 0),
129+
$this->makeMockShare('11', 103, 'user1', '/share7', 31),
130+
$this->makeMockShare('12', 103, 'user2', '/share7', 31),
131+
$this->makeMockShare('13', 103, 'user2', '/share7', 31),
136132
];
137133
// tests regarding circles and sciencemesh are made in the apps themselves.
138134
$circleShares = [];
@@ -198,10 +194,10 @@ public static function mergeSharesDataProvider(): array {
198194
// #0: share as outsider with "group1" and "user1" with same permissions
199195
[
200196
[
201-
[1, 100, 'user2', '/share2', 31, null],
197+
['1', 100, 'user2', '/share2', 31, null],
202198
],
203199
[
204-
[2, 100, 'user2', '/share2', 31, null],
200+
['2', 100, 'user2', '/share2', 31, null],
205201
],
206202
[
207203
// combined, user share has higher priority
@@ -211,10 +207,10 @@ public static function mergeSharesDataProvider(): array {
211207
// #1: share as outsider with "group1" and "user1" with different permissions
212208
[
213209
[
214-
[1, 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => true], ['scope' => 'app', 'key' => 'attribute1', 'value' => true]]],
210+
['1', 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => true], ['scope' => 'app', 'key' => 'attribute1', 'value' => true]]],
215211
],
216212
[
217-
[2, 100, 'user2', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => false], ['scope' => 'app', 'key' => 'attribute2', 'value' => false]]],
213+
['2', 100, 'user2', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => false], ['scope' => 'app', 'key' => 'attribute2', 'value' => false]]],
218214
],
219215
[
220216
// use highest permissions
@@ -226,8 +222,8 @@ public static function mergeSharesDataProvider(): array {
226222
[
227223
],
228224
[
229-
[1, 100, 'user2', '/share', 31, null],
230-
[2, 100, 'user2', '/share', 31, []],
225+
['1', 100, 'user2', '/share', 31, null],
226+
['2', 100, 'user2', '/share', 31, []],
231227
],
232228
[
233229
// combined, first group share has higher priority
@@ -239,8 +235,8 @@ public static function mergeSharesDataProvider(): array {
239235
[
240236
],
241237
[
242-
[1, 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => false]]],
243-
[2, 100, 'user2', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => true]]],
238+
['1', 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => false]]],
239+
['2', 100, 'user2', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => true]]],
244240
],
245241
[
246242
// use higher permissions (most permissive)
@@ -252,7 +248,7 @@ public static function mergeSharesDataProvider(): array {
252248
[
253249
],
254250
[
255-
[1, 100, 'user1', '/share', 31, []],
251+
['1', 100, 'user1', '/share', 31, []],
256252
],
257253
[
258254
// no received share since "user1" is the sharer/owner
@@ -263,8 +259,8 @@ public static function mergeSharesDataProvider(): array {
263259
[
264260
],
265261
[
266-
[1, 100, 'user1', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => true]]],
267-
[2, 100, 'user1', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => false]]],
262+
['1', 100, 'user1', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => true]]],
263+
['2', 100, 'user1', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => false]]],
268264
],
269265
[
270266
// no received share since "user1" is the sharer/owner
@@ -275,7 +271,7 @@ public static function mergeSharesDataProvider(): array {
275271
[
276272
],
277273
[
278-
[1, 100, 'user2', '/share', 0, []],
274+
['1', 100, 'user2', '/share', 0, []],
279275
],
280276
[
281277
// no received share since "user1" opted out
@@ -284,10 +280,10 @@ public static function mergeSharesDataProvider(): array {
284280
// #7: share as outsider with "group1" and "user1" where recipient renamed in between
285281
[
286282
[
287-
[1, 100, 'user2', '/share2-renamed', 31, []],
283+
['1', 100, 'user2', '/share2-renamed', 31, []],
288284
],
289285
[
290-
[2, 100, 'user2', '/share2', 31, []],
286+
['2', 100, 'user2', '/share2', 31, []],
291287
],
292288
[
293289
// use target of least recent share
@@ -297,10 +293,10 @@ public static function mergeSharesDataProvider(): array {
297293
// #8: share as outsider with "group1" and "user1" where recipient renamed in between
298294
[
299295
[
300-
[2, 100, 'user2', '/share2', 31, []],
296+
['2', 100, 'user2', '/share2', 31, []],
301297
],
302298
[
303-
[1, 100, 'user2', '/share2-renamed', 31, []],
299+
['1', 100, 'user2', '/share2-renamed', 31, []],
304300
],
305301
[
306302
// use target of least recent share
@@ -310,10 +306,10 @@ public static function mergeSharesDataProvider(): array {
310306
// #9: share as outsider with "nullgroup" and "user1" where recipient renamed in between
311307
[
312308
[
313-
[2, 100, 'user2', '/share2', 31, []],
309+
['2', 100, 'user2', '/share2', 31, []],
314310
],
315311
[
316-
[1, 100, 'nullgroup', '/share2-renamed', 31, []],
312+
['1', 100, 'nullgroup', '/share2-renamed', 31, []],
317313
],
318314
[
319315
// use target of least recent share
@@ -355,7 +351,6 @@ public function testMergeShares(array $userShares, array $groupShares, array $ex
355351
$circleShares = [];
356352
$roomShares = [];
357353
$deckShares = [];
358-
$scienceMeshShares = [];
359354
$this->shareManager->expects($this->exactly(5))
360355
->method('getSharedWith')
361356
->willReturnMap([
@@ -384,7 +379,7 @@ public function testMergeShares(array $userShares, array $groupShares, array $ex
384379

385380
foreach ($mounts as $index => $mount) {
386381
$expectedShare = $expectedShares[$index];
387-
$this->assertInstanceOf('OCA\Files_Sharing\SharedMount', $mount);
382+
$this->assertInstanceOf(SharedMount::class, $mount);
388383

389384
// supershare
390385
/** @var SharedMount $mount */

0 commit comments

Comments
 (0)