Skip to content

refactor(IShare): Add typing for IShare #57575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
CarlSchwan wants to merge 1 commit into
base: master
Choose a base branch
from
+178 −237
Open

refactor(IShare): Add typing for IShare #57575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/federatedfilesharing/tests/Controller/MountPublicLinkControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ protected function setUp(): void {
$this->rootFolder = $this->createMock(IRootFolder::class);
$this->userManager = $this->createMock(IUserManager::class);
$this->share = new Share($this->rootFolder, $this->userManager);
$this->share->setId('42');
$this->session = $this->createMock(ISession::class);
$this->l10n = $this->createMock(IL10N::class);
$this->userSession = $this->createMock(IUserSession::class);
Expand Down
49 changes: 19 additions & 30 deletions apps/federatedfilesharing/tests/FederatedShareProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/
namespace OCA\FederatedFileSharing\Tests;

use LogicException;
use OC\Federation\CloudIdManager;
use OCA\FederatedFileSharing\AddressHandler;
use OCA\FederatedFileSharing\FederatedShareProvider;
Expand Down Expand Up @@ -250,16 +251,12 @@ public function testCreateCouldNotFindServer(): void {
$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());
}

$qb = $this->connection->getQueryBuilder();
$stmt = $qb->select('*')
->from('share')
->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId())))
->executeQuery();

$data = $stmt->fetchAssociative();
$stmt->closeCursor();

$this->assertFalse($data);
try {
$share->getId();
$this->fail();
} catch (LogicException $e) {
// object is expected to not have been created
}
}

public function testCreateException(): void {
Expand Down Expand Up @@ -311,16 +308,12 @@ public function testCreateException(): void {
$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());
}

$qb = $this->connection->getQueryBuilder();
$stmt = $qb->select('*')
->from('share')
->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId())))
->executeQuery();

$data = $stmt->fetchAssociative();
$stmt->closeCursor();

$this->assertFalse($data);
try {
$share->getId();
$this->fail();
} catch (LogicException $e) {
// object is expected to not have been created
}
}

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

$qb = $this->connection->getQueryBuilder();
$stmt = $qb->select('*')
->from('share')
->where($qb->expr()->eq('id', $qb->createNamedParameter($share->getId())))
->executeQuery();

$data = $stmt->fetchAssociative();
$stmt->closeCursor();

$this->assertFalse($data);
try {
$share->getId();
$this->fail();
} catch (LogicException $e) {
// object is expected to not have been created
}
}

public function testCreateAlreadyShared(): void {
Expand Down
14 changes: 7 additions & 7 deletions apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ public function testGetGetShareNotExists() {
*/

public function createShare(
int $id,
string $id,
int $shareType,
?string $sharedWith,
string $sharedBy,
Expand Down Expand Up @@ -658,7 +658,7 @@ public static function dataGetShare(): array {

// File shared with user
$share = [
100,
'100',
IShare::TYPE_USER,
'userId',
'initiatorId',
Expand All @@ -677,7 +677,7 @@ public static function dataGetShare(): array {
[],
];
$expected = [
'id' => 100,
'id' => '100',
'share_type' => IShare::TYPE_USER,
'share_with' => 'userId',
'share_with_displayname' => 'userDisplay',
Expand Down Expand Up @@ -717,7 +717,7 @@ public static function dataGetShare(): array {

// Folder shared with group
$share = [
101,
'101',
IShare::TYPE_GROUP,
'groupId',
'initiatorId',
Expand All @@ -736,7 +736,7 @@ public static function dataGetShare(): array {
[],
];
$expected = [
'id' => 101,
'id' => '101',
'share_type' => IShare::TYPE_GROUP,
'share_with' => 'groupId',
'share_with_displayname' => 'groupId',
Expand Down Expand Up @@ -776,7 +776,7 @@ public static function dataGetShare(): array {
// File shared by link with Expire
$expire = \DateTime::createFromFormat('Y-m-d h:i:s', '2000-01-02 01:02:03');
$share = [
101,
'101',
IShare::TYPE_LINK,
null,
'initiatorId',
Expand All @@ -794,7 +794,7 @@ public static function dataGetShare(): array {
'first link share'
];
$expected = [
'id' => 101,
'id' => '101',
'share_type' => IShare::TYPE_LINK,
'password' => 'password',
'share_with' => 'password',
Expand Down
84 changes: 40 additions & 44 deletions apps/files_sharing/tests/MountProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ class MountProviderTest extends \Test\TestCase {
protected function setUp(): void {
parent::setUp();

$this->config = $this->getMockBuilder(IConfig::class)->getMock();
$this->user = $this->getMockBuilder(IUser::class)->getMock();
$this->loader = $this->getMockBuilder(IStorageFactory::class)->getMock();
$this->shareManager = $this->getMockBuilder(IManager::class)->getMock();
$this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock();
$this->config = $this->createMock(IConfig::class);
$this->user = $this->createMock(IUser::class);
$this->loader = $this->createMock(IStorageFactory::class);
$this->shareManager = $this->createMock(IManager::class);
$this->logger = $this->createMock(LoggerInterface::class);
$eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->cache = $this->createMock(ICache::class);
$this->cache->method('get')->willReturn(true);
Expand All @@ -58,11 +58,7 @@ protected function setUp(): void {
$this->provider = new MountProvider($this->config, $this->shareManager, $this->logger, $eventDispatcher, $cacheFactory, $mountManager);
}

private function makeMockShareAttributes($attrs) {
if ($attrs === null) {
return null;
}

private function makeMockShareAttributes(array $attrs): IShareAttributes&MockObject {
$shareAttributes = $this->createMock(IShareAttributes::class);
$shareAttributes->method('toArray')->willReturn($attrs);
$shareAttributes->method('getAttribute')->willReturnCallback(
Expand All @@ -79,14 +75,14 @@ function ($scope, $key) use ($attrs) {
return $shareAttributes;
}

private function makeMockShare($id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $attributes = null) {
private function makeMockShare(string $id, $nodeId, $owner = 'user2', $target = null, $permissions = 31, $attributes = null) {
$share = $this->createMock(IShare::class);
$share->expects($this->any())
->method('getPermissions')
->willReturn($permissions);
$share->expects($this->any())
->method('getAttributes')
->willReturn($this->makeMockShareAttributes($attributes));
->willReturn($attributes === null ? null : $this->makeMockShareAttributes($attributes));
$share->expects($this->any())
->method('getShareOwner')
->willReturn($owner);
Expand Down Expand Up @@ -119,25 +115,25 @@ public function testExcludeShares(): void {
$attr1 = [];
$attr2 = [['scope' => 'permission', 'key' => 'download', 'value' => true]];
$userShares = [
$this->makeMockShare(1, 100, 'user2', '/share2', 0, $attr1),
$this->makeMockShare(2, 100, 'user2', '/share2', 31, $attr2),
$this->makeMockShare('1', 100, 'user2', '/share2', 0, $attr1),
$this->makeMockShare('2', 100, 'user2', '/share2', 31, $attr2),
];
$groupShares = [
$this->makeMockShare(3, 100, 'user2', '/share2', 0, $attr1),
$this->makeMockShare(4, 101, 'user2', '/share4', 31, $attr2),
$this->makeMockShare(5, 100, 'user1', '/share4', 31, $attr2),
$this->makeMockShare('3', 100, 'user2', '/share2', 0, $attr1),
$this->makeMockShare('4', 101, 'user2', '/share4', 31, $attr2),
$this->makeMockShare('5', 100, 'user1', '/share4', 31, $attr2),
];
$roomShares = [
$this->makeMockShare(6, 102, 'user2', '/share6', 0),
$this->makeMockShare(7, 102, 'user1', '/share6', 31),
$this->makeMockShare(8, 102, 'user2', '/share6', 31),
$this->makeMockShare(9, 102, 'user2', '/share6', 31),
$this->makeMockShare('6', 102, 'user2', '/share6', 0),
$this->makeMockShare('7', 102, 'user1', '/share6', 31),
$this->makeMockShare('8', 102, 'user2', '/share6', 31),
$this->makeMockShare('9', 102, 'user2', '/share6', 31),
];
$deckShares = [
$this->makeMockShare(10, 103, 'user2', '/share7', 0),
$this->makeMockShare(11, 103, 'user1', '/share7', 31),
$this->makeMockShare(12, 103, 'user2', '/share7', 31),
$this->makeMockShare(13, 103, 'user2', '/share7', 31),
$this->makeMockShare('10', 103, 'user2', '/share7', 0),
$this->makeMockShare('11', 103, 'user1', '/share7', 31),
$this->makeMockShare('12', 103, 'user2', '/share7', 31),
$this->makeMockShare('13', 103, 'user2', '/share7', 31),
];
// tests regarding circles and sciencemesh are made in the apps themselves.
$circleShares = [];
Expand Down Expand Up @@ -203,10 +199,10 @@ public static function mergeSharesDataProvider(): array {
// #0: share as outsider with "group1" and "user1" with same permissions
[
[
[1, 100, 'user2', '/share2', 31, null],
['1', 100, 'user2', '/share2', 31, null],
],
[
[2, 100, 'user2', '/share2', 31, null],
['2', 100, 'user2', '/share2', 31, null],
],
[
// combined, user share has higher priority
Expand All @@ -216,10 +212,10 @@ public static function mergeSharesDataProvider(): array {
// #1: share as outsider with "group1" and "user1" with different permissions
[
[
[1, 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => true], ['scope' => 'app', 'key' => 'attribute1', 'value' => true]]],
['1', 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => true], ['scope' => 'app', 'key' => 'attribute1', 'value' => true]]],
],
[
[2, 100, 'user2', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => false], ['scope' => 'app', 'key' => 'attribute2', 'value' => false]]],
['2', 100, 'user2', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => false], ['scope' => 'app', 'key' => 'attribute2', 'value' => false]]],
],
[
// use highest permissions
Expand All @@ -231,8 +227,8 @@ public static function mergeSharesDataProvider(): array {
[
],
[
[1, 100, 'user2', '/share', 31, null],
[2, 100, 'user2', '/share', 31, []],
['1', 100, 'user2', '/share', 31, null],
['2', 100, 'user2', '/share', 31, []],
],
[
// combined, first group share has higher priority
Expand All @@ -244,8 +240,8 @@ public static function mergeSharesDataProvider(): array {
[
],
[
[1, 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => false]]],
[2, 100, 'user2', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => true]]],
['1', 100, 'user2', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => false]]],
['2', 100, 'user2', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => true]]],
],
[
// use higher permissions (most permissive)
Expand All @@ -257,7 +253,7 @@ public static function mergeSharesDataProvider(): array {
[
],
[
[1, 100, 'user1', '/share', 31, []],
['1', 100, 'user1', '/share', 31, []],
],
[
// no received share since "user1" is the sharer/owner
Expand All @@ -268,8 +264,8 @@ public static function mergeSharesDataProvider(): array {
[
],
[
[1, 100, 'user1', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => true]]],
[2, 100, 'user1', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => false]]],
['1', 100, 'user1', '/share', 31, [['scope' => 'permission', 'key' => 'download', 'value' => true]]],
['2', 100, 'user1', '/share', 15, [['scope' => 'permission', 'key' => 'download', 'value' => false]]],
],
[
// no received share since "user1" is the sharer/owner
Expand All @@ -280,7 +276,7 @@ public static function mergeSharesDataProvider(): array {
[
],
[
[1, 100, 'user2', '/share', 0, []],
['1', 100, 'user2', '/share', 0, []],
],
[
// no received share since "user1" opted out
Expand All @@ -289,10 +285,10 @@ public static function mergeSharesDataProvider(): array {
// #7: share as outsider with "group1" and "user1" where recipient renamed in between
[
[
[1, 100, 'user2', '/share2-renamed', 31, []],
['1', 100, 'user2', '/share2-renamed', 31, []],
],
[
[2, 100, 'user2', '/share2', 31, []],
['2', 100, 'user2', '/share2', 31, []],
],
[
// use target of least recent share
Expand All @@ -302,10 +298,10 @@ public static function mergeSharesDataProvider(): array {
// #8: share as outsider with "group1" and "user1" where recipient renamed in between
[
[
[2, 100, 'user2', '/share2', 31, []],
['2', 100, 'user2', '/share2', 31, []],
],
[
[1, 100, 'user2', '/share2-renamed', 31, []],
['1', 100, 'user2', '/share2-renamed', 31, []],
],
[
// use target of least recent share
Expand All @@ -315,10 +311,10 @@ public static function mergeSharesDataProvider(): array {
// #9: share as outsider with "nullgroup" and "user1" where recipient renamed in between
[
[
[2, 100, 'user2', '/share2', 31, []],
['2', 100, 'user2', '/share2', 31, []],
],
[
[1, 100, 'nullgroup', '/share2-renamed', 31, []],
['1', 100, 'nullgroup', '/share2-renamed', 31, []],
],
[
// use target of least recent share
Expand Down Expand Up @@ -388,7 +384,7 @@ public function testMergeShares(array $userShares, array $groupShares, array $ex

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

// supershare
/** @var SharedMount $mount */
Expand Down
Loading
Loading