upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-02-03 20:41:22 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 33

refactor(IShare): Add typing for node

Browse source 
This might also improve a bit the performance.

Signed-off-by: Carl Schwan <carlschwan@kde.org>
This commit is contained in:
Carl Schwan 2026-01-15 17:32:18 +01:00
parent 3950ef8b16
commit c8989d853c
No known key found for this signature in database
GPG key ID: 02325448204E452A
13 changed files with 166 additions and 237 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
apps/federatedfilesharing/tests/Controller/MountPublicLinkControllerTest.php
View file

@ -58,6 +58,7 @@ class MountPublicLinkControllerTest extends \Test\TestCase {
$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);

37
apps/federatedfilesharing/tests/FederatedShareProviderTest.php
View file

@ -8,6 +8,7 @@ declare(strict_types=1);
*/
namespace OCA\FederatedFileSharing\Tests;
use LogicException;
use OC\Federation\CloudIdManager;
use OCA\FederatedFileSharing\AddressHandler;
use OCA\FederatedFileSharing\FederatedShareProvider;
@ -202,6 +203,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
}
public function testCreateCouldNotFindServer(): void {
$this->expectException(LogicException::class);
$share = $this->shareManager->newShare();
$node = $this->createMock(File::class);
@ -250,19 +252,11 @@ class FederatedShareProviderTest extends \Test\TestCase {
$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);
$share->getId();
}
public function testCreateException(): void {
$this->expectException(LogicException::class);
$share = $this->shareManager->newShare();
$node = $this->createMock(File::class);
@ -311,19 +305,11 @@ class FederatedShareProviderTest extends \Test\TestCase {
$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);
$share->getId();
}
public function testCreateShareWithSelf(): void {
$this->expectException(LogicException::class);
$share = $this->shareManager->newShare();
$node = $this->createMock(File::class);
@ -354,16 +340,7 @@ class FederatedShareProviderTest extends \Test\TestCase {
$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);
$share->getId();
}
public function testCreateAlreadyShared(): void {

14
apps/files_sharing/tests/Controller/ShareAPIControllerTest.php
View file

@ -592,7 +592,7 @@ class ShareAPIControllerTest extends TestCase {
*/
public function createShare(
int $id,
string $id,
int $shareType,
?string $sharedWith,
string $sharedBy,
@ -658,7 +658,7 @@ class ShareAPIControllerTest extends TestCase {
// File shared with user
$share = [
100,
'100',
IShare::TYPE_USER,
'userId',
'initiatorId',
@ -677,7 +677,7 @@ class ShareAPIControllerTest extends TestCase {
[],
];
$expected = [
'id' => 100,
'id' => '100',
'share_type' => IShare::TYPE_USER,
'share_with' => 'userId',
'share_with_displayname' => 'userDisplay',
@ -717,7 +717,7 @@ class ShareAPIControllerTest extends TestCase {
// Folder shared with group
$share = [
101,
'101',
IShare::TYPE_GROUP,
'groupId',
'initiatorId',
@ -736,7 +736,7 @@ class ShareAPIControllerTest extends TestCase {
[],
];
$expected = [
'id' => 101,
'id' => '101',
'share_type' => IShare::TYPE_GROUP,
'share_with' => 'groupId',
'share_with_displayname' => 'groupId',
@ -776,7 +776,7 @@ class ShareAPIControllerTest extends TestCase {
// 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',
@ -794,7 +794,7 @@ class ShareAPIControllerTest extends TestCase {
'first link share'
];
$expected = [
'id' => 101,
'id' => '101',
'share_type' => IShare::TYPE_LINK,
'password' => 'password',
'share_with' => 'password',

84
apps/files_sharing/tests/MountProviderTest.php
View file

@ -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);
@ -58,11 +58,7 @@ class MountProviderTest extends \Test\TestCase {
$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(
@ -79,14 +75,14 @@ class MountProviderTest extends \Test\TestCase {
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);
@ -119,25 +115,25 @@ class MountProviderTest extends \Test\TestCase {
$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 = [];
@ -203,10 +199,10 @@ class MountProviderTest extends \Test\TestCase {
// #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
@ -216,10 +212,10 @@ class MountProviderTest extends \Test\TestCase {
// #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
@ -231,8 +227,8 @@ class MountProviderTest extends \Test\TestCase {
[
],
[
[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
@ -244,8 +240,8 @@ class MountProviderTest extends \Test\TestCase {
[
],
[
[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)
@ -257,7 +253,7 @@ class MountProviderTest extends \Test\TestCase {
[
],
[
[1, 100, 'user1', '/share', 31, []],
['1', 100, 'user1', '/share', 31, []],
],
[
// no received share since "user1" is the sharer/owner
@ -268,8 +264,8 @@ class MountProviderTest extends \Test\TestCase {
[
],
[
[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
@ -280,7 +276,7 @@ class MountProviderTest extends \Test\TestCase {
[
],
[
[1, 100, 'user2', '/share', 0, []],
['1', 100, 'user2', '/share', 0, []],
],
[
// no received share since "user1" opted out
@ -289,10 +285,10 @@ class MountProviderTest extends \Test\TestCase {
// #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
@ -302,10 +298,10 @@ class MountProviderTest extends \Test\TestCase {
// #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
@ -315,10 +311,10 @@ class MountProviderTest extends \Test\TestCase {
// #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
@ -388,7 +384,7 @@ class MountProviderTest extends \Test\TestCase {
foreach ($mounts as $index => $mount) {
$expectedShare = $expectedShares[$index];
$this->assertInstanceOf('OCA\Files_Sharing\SharedMount', $mount);
$this->assertInstanceOf(SharedMount::class, $mount);
// supershare
/** @var SharedMount $mount */

13
apps/sharebymail/lib/ShareByMailProvider.php
View file

@ -218,7 +218,7 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
/**
* @throws \Exception
*/
protected function createMailShare(IShare $share): int {
protected function createMailShare(IShare $share): string {
$share->setToken($this->generateToken());
return $this->addShareToDB(
$share->getNodeId(),
@ -678,7 +678,7 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
?string $note = '',
?IAttributes $attributes = null,
?bool $mailSend = true,
): int {
): string {
$qb = $this->dbConnection->getQueryBuilder();
$qb->insert('share')
->setValue('share_type', $qb->createNamedParameter(IShare::TYPE_EMAIL))
@ -708,7 +708,7 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
}
$qb->executeStatement();
return $qb->getLastInsertId();
return (string)$qb->getLastInsertId();
}
/**
@ -872,6 +872,8 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
throw new ShareNotFound();
}
$data['id'] = (string)$data['id'];
try {
$share = $this->createShareObject($data);
} catch (InvalidShare $e) {
@ -897,6 +899,7 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
$shares = [];
while ($data = $cursor->fetchAssociative()) {
$data['id'] = (string)$data['id'];
$shares[] = $this->createShareObject($data);
}
$cursor->closeCursor();
@ -964,6 +967,8 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
throw new ShareNotFound('Share not found', $this->l->t('Could not find share'));
}
$data['id'] = (string)$data['id'];
try {
$share = $this->createShareObject($data);
} catch (InvalidShare $e) {
@ -1091,7 +1096,7 @@ class ShareByMailProvider extends DefaultShareProvider implements IShareProvider
*
* @throws ShareNotFound
*/
protected function getRawShare(int $id): array {
protected function getRawShare(string $id): array {
// Now fetch the inserted share and create a complete share object
$qb = $this->dbConnection->getQueryBuilder();
$qb->select('*')

86
apps/sharebymail/tests/ShareByMailProviderTest.php
View file

@ -171,9 +171,9 @@ class ShareByMailProviderTest extends TestCase {
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'sendEmail', 'sendPassword']);
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn('42');
$instance->expects($this->once())->method('createShareActivity')->with($share);
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn(['rawShare']);
$instance->expects($this->once())->method('getRawShare')->with('42')->willReturn(['rawShare']);
$instance->expects($this->once())->method('createShareObject')->with(['rawShare'])->willReturn($expectedShare);
$share->expects($this->any())->method('getNode')->willReturn($node);
@ -199,15 +199,15 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity', 'sendEmail', 'sendPassword', 'sendPasswordToOwner']);
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn('42');
$instance->expects($this->once())->method('createShareActivity')->with($share);
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn(['rawShare']);
$instance->expects($this->once())->method('getRawShare')->with('42')->willReturn(['rawShare']);
$instance->expects($this->once())->method('createShareObject')->with(['rawShare'])->willReturn($expectedShare);
$share->expects($this->any())->method('getNode')->willReturn($node);
@ -238,16 +238,16 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity', 'sendEmail', 'sendPassword', 'sendPasswordToOwner']);
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn('42');
$instance->expects($this->once())->method('createShareActivity')->with($share);
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn(['rawShare', 'password' => 'password']);
$instance->expects($this->once())->method('getRawShare')->with('42')->willReturn(['rawShare', 'password' => 'password']);
$instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'password'])->willReturn($expectedShare);
$share->expects($this->any())->method('getNode')->willReturn($node);
@ -281,7 +281,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');
@ -292,9 +292,9 @@ class ShareByMailProviderTest extends TestCase {
]);
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn('42');
$instance->expects($this->once())->method('createShareActivity')->with($share);
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn(['rawShare', 'password' => 'password']);
$instance->expects($this->once())->method('getRawShare')->with('42')->willReturn(['rawShare', 'password' => 'password']);
$instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'password'])->willReturn($expectedShare);
$share->expects($this->any())->method('getNode')->willReturn($node);
@ -331,7 +331,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');
@ -350,9 +350,9 @@ class ShareByMailProviderTest extends TestCase {
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'createPasswordSendActivity', 'sendPasswordToOwner']);
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn('42');
$instance->expects($this->once())->method('createShareActivity')->with($share);
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn(['rawShare', 'password' => 'autogeneratedPassword']);
$instance->expects($this->once())->method('getRawShare')->with('42')->willReturn(['rawShare', 'password' => 'autogeneratedPassword']);
$instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'autogeneratedPassword'])->willReturn($expectedShare);
// Initially not set, but will be set by the autoGeneratePassword method.
@ -419,7 +419,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');
@ -430,9 +430,9 @@ class ShareByMailProviderTest extends TestCase {
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity', 'sendPasswordToOwner']);
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn('42');
$instance->expects($this->once())->method('createShareActivity')->with($share);
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn(['rawShare', 'password' => 'password']);
$instance->expects($this->once())->method('getRawShare')->with('42')->willReturn(['rawShare', 'password' => 'password']);
$instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'password'])->willReturn($expectedShare);
$share->expects($this->exactly(3))->method('getPassword')->willReturn('password');
@ -507,7 +507,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(true);
$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');
@ -518,9 +518,9 @@ class ShareByMailProviderTest extends TestCase {
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity']);
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn('42');
$instance->expects($this->once())->method('createShareActivity')->with($share);
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn(['rawShare', 'password' => 'autogeneratedPassword']);
$instance->expects($this->once())->method('getRawShare')->with('42')->willReturn(['rawShare', 'password' => 'autogeneratedPassword']);
$instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'autogeneratedPassword'])->willReturn($expectedShare);
$share->expects($this->exactly(4))->method('getPassword')->willReturnOnConsecutiveCalls(null, 'autogeneratedPassword', 'autogeneratedPassword', 'autogeneratedPassword');
@ -598,7 +598,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn(false);
$share->expects($this->any())->method('getSharedBy')->willReturn('owner');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');
@ -613,9 +613,9 @@ class ShareByMailProviderTest extends TestCase {
$instance = $this->getInstance(['getSharedWith', 'createMailShare', 'getRawShare', 'createShareObject', 'createShareActivity', 'autoGeneratePassword', 'createPasswordSendActivity', 'sendEmail', 'sendPassword', 'sendPasswordToOwner']);
$instance->expects($this->once())->method('getSharedWith')->willReturn([]);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn(42);
$instance->expects($this->once())->method('createMailShare')->with($share)->willReturn('42');
$instance->expects($this->once())->method('createShareActivity')->with($share);
$instance->expects($this->once())->method('getRawShare')->with(42)->willReturn(['rawShare', 'password' => 'password']);
$instance->expects($this->once())->method('getRawShare')->with('42')->willReturn(['rawShare', 'password' => 'password']);
$instance->expects($this->once())->method('createShareObject')->with(['rawShare', 'password' => 'password'])->willReturn($expectedShare);
$share->expects($this->any())->method('getNode')->willReturn($node);
@ -684,12 +684,12 @@ class ShareByMailProviderTest extends TestCase {
$instance = $this->getInstance(['generateToken', 'addShareToDB', 'sendMailNotification']);
$instance->expects($this->once())->method('generateToken')->willReturn('token');
$instance->expects($this->once())->method('addShareToDB')->willReturn(42);
$instance->expects($this->once())->method('addShareToDB')->willReturn('42');
// The manager handle the mail sending
$instance->expects($this->never())->method('sendMailNotification');
$this->assertSame(42,
$this->assertSame('42',
$this->invokePrivate($instance, 'createMailShare', [$this->share])
);
}
@ -837,14 +837,14 @@ class ShareByMailProviderTest extends TestCase {
$originalShare = $this->createMock(IShare::class);
$originalShare->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
$originalShare->expects($this->any())->method('getNode')->willReturn($node);
$originalShare->expects($this->any())->method('getId')->willReturn(42);
$originalShare->expects($this->any())->method('getId')->willReturn('42');
$originalShare->expects($this->any())->method('getPassword')->willReturn($originalPassword);
$originalShare->expects($this->any())->method('getSendPasswordByTalk')->willReturn($originalSendPasswordByTalk);
$share = $this->createMock(IShare::class);
$share->expects($this->any())->method('getSharedWith')->willReturn('receiver@example.com');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getPassword')->willReturn($newPassword);
$share->expects($this->any())->method('getSendPasswordByTalk')->willReturn($newSendPasswordByTalk);
@ -871,8 +871,8 @@ class ShareByMailProviderTest extends TestCase {
public function testDelete(): void {
$instance = $this->getInstance(['removeShareFromTable', 'createShareActivity']);
$this->share->expects($this->once())->method('getId')->willReturn(42);
$instance->expects($this->once())->method('removeShareFromTable')->with(42);
$this->share->expects($this->once())->method('getId')->willReturn('42');
$instance->expects($this->once())->method('removeShareFromTable')->with('42');
$instance->expects($this->once())->method('createShareActivity')->with($this->share, 'unshare');
$instance->delete($this->share);
}
@ -896,7 +896,7 @@ class ShareByMailProviderTest extends TestCase {
function ($data) use ($uidOwner, $sharedBy, $id2) {
$this->assertSame($uidOwner, $data['uid_owner']);
$this->assertSame($sharedBy, $data['uid_initiator']);
$this->assertSame($id2, (int)$data['id']);
$this->assertSame($id2, (string)$data['id']);
return $this->share;
}
);
@ -948,7 +948,7 @@ class ShareByMailProviderTest extends TestCase {
function ($data) use ($uidOwner, $sharedBy, $id) {
$this->assertSame($uidOwner, $data['uid_owner']);
$this->assertSame($sharedBy, $data['uid_initiator']);
$this->assertSame($id, (int)$data['id']);
$this->assertSame($id, $data['id']);
return $this->share;
}
);
@ -979,7 +979,7 @@ class ShareByMailProviderTest extends TestCase {
$instance->expects($this->once())->method('createShareObject')
->willReturnCallback(
function ($data) use ($idMail) {
$this->assertSame($idMail, (int)$data['id']);
$this->assertSame($idMail, $data['id']);
return $this->share;
}
);
@ -1088,7 +1088,7 @@ class ShareByMailProviderTest extends TestCase {
$this->assertTrue(is_array($after));
$this->assertSame(1, count($after));
$this->assertSame($id, (int)$after[0]['id']);
$this->assertSame($id, (string)$after[0]['id']);
}
public function testGetRawShare(): void {
@ -1132,10 +1132,10 @@ class ShareByMailProviderTest extends TestCase {
$id = $this->createDummyShare($itemType, $itemSource, $shareWith, $sharedBy, $uidOwner, $permissions, $token);
$this->invokePrivate($instance, 'getRawShare', [$id + 1]);
$this->invokePrivate($instance, 'getRawShare', [(string)((int)$id + 1)]);
}
private function createDummyShare($itemType, $itemSource, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $note = '', $shareType = IShare::TYPE_EMAIL) {
private function createDummyShare($itemType, $itemSource, $shareWith, $sharedBy, $uidOwner, $permissions, $token, $note = '', $shareType = IShare::TYPE_EMAIL): string {
$qb = $this->connection->getQueryBuilder();
$qb->insert('share')
->setValue('share_type', $qb->createNamedParameter($shareType))
@ -1157,9 +1157,7 @@ class ShareByMailProviderTest extends TestCase {
$qb->setValue('file_target', $qb->createNamedParameter(''));
$qb->executeStatement();
$id = $qb->getLastInsertId();
return (int)$id;
return (string)$qb->getLastInsertId();
}
public function testGetSharesInFolder(): void {
@ -1367,7 +1365,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSharedBy')->willReturn('OwnerUser');
$share->expects($this->any())->method('getSharedWith')->willReturn('john@doe.com');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');
@ -1486,7 +1484,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSharedBy')->willReturn('OwnerUser');
$share->expects($this->any())->method('getSharedWith')->willReturn('john@doe.com');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('This is a note to the recipient');
$share->expects($this->any())->method('getToken')->willReturn('token');
@ -1610,7 +1608,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSharedBy')->willReturn('OwnerUser');
$share->expects($this->any())->method('getSharedWith')->willReturn('john@doe.com');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getExpirationDate')->willReturn($expiration);
$share->expects($this->any())->method('getToken')->willReturn('token');
@ -1705,7 +1703,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSharedBy')->willReturn('InitiatorUser');
$share->expects($this->any())->method('getSharedWith')->willReturn('john@doe.com');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');
@ -1803,7 +1801,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSharedBy')->willReturn('OwnerUser');
$share->expects($this->any())->method('getSharedWith')->willReturn('john@doe.com');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');
@ -1897,7 +1895,7 @@ class ShareByMailProviderTest extends TestCase {
$share->expects($this->any())->method('getSharedBy')->willReturn('InitiatorUser');
$share->expects($this->any())->method('getSharedWith')->willReturn('john@doe.com');
$share->expects($this->any())->method('getNode')->willReturn($node);
$share->expects($this->any())->method('getId')->willReturn(42);
$share->expects($this->any())->method('getId')->willReturn('42');
$share->expects($this->any())->method('getNote')->willReturn('');
$share->expects($this->any())->method('getToken')->willReturn('token');

8
build/psalm-baseline.xml
View file

@ -4125,14 +4125,6 @@
<code><![CDATA[getProviderForType]]></code>
</InvalidReturnType>
</file>
<file src="lib/private/Share20/Share.php">
<LessSpecificReturnStatement>
<code><![CDATA[$this->node]]></code>
</LessSpecificReturnStatement>
<MoreSpecificReturnType>
<code><![CDATA[getNode]]></code>
</MoreSpecificReturnType>
</file>
<file src="lib/private/Streamer.php">
<UndefinedInterfaceMethod>
<code><![CDATA[get]]></code>

4
lib/private/Share20/Manager.php
View file

@ -185,7 +185,9 @@ class Manager implements IManager {
}
// The path should be set
if ($share->getNode() === null) {
try {
$share->getNode();
} catch (NotFoundException $e) {
throw new \InvalidArgumentException($this->l->t('Shared path must be set'));
}

67
lib/private/Share20/Share.php
View file

@ -19,14 +19,12 @@ use OCP\Share\Exceptions\IllegalIDChangeException;
use OCP\Share\IAttributes;
use OCP\Share\IManager;
use OCP\Share\IShare;
use Override;
class Share implements IShare {
/** @var string */
private $id;
/** @var string */
private $providerId;
/** @var Node */
private $node;
private ?string $id = null;
private ?string $providerId = null;
private ?Node $node = null;
/** @var int */
private $fileId;
/** @var string */
@ -82,74 +80,51 @@ class Share implements IShare {
) {
}
/**
* @inheritdoc
*/
public function setId($id) {
/** @var mixed $id Let's be safe until strong typing */
if (is_int($id)) {
$id = (string)$id;
}
if (!is_string($id)) {
throw new \InvalidArgumentException('String expected.');
}
#[Override]
public function setId(string $id): self {
if ($this->id !== null) {
throw new IllegalIDChangeException('Not allowed to assign a new internal id to a share');
}
$this->id = trim($id);
$this->id = $id;
return $this;
}
/**
* @inheritdoc
*/
public function getId() {
#[Override]
public function getId(): string {
if ($this->id === null) {
throw new \LogicException('Share id is null');
}
return $this->id;
}
/**
* @inheritdoc
*/
public function getFullId() {
#[Override]
public function getFullId(): string {
if ($this->providerId === null || $this->id === null) {
throw new \UnexpectedValueException;
}
return $this->providerId . ':' . $this->id;
}
/**
* @inheritdoc
*/
public function setProviderId($id) {
if (!is_string($id)) {
throw new \InvalidArgumentException('String expected.');
}
#[Override]
public function setProviderId(string $id): self {
if ($this->providerId !== null) {
throw new IllegalIDChangeException('Not allowed to assign a new provider id to a share');
}
$this->providerId = trim($id);
$this->providerId = $id;
return $this;
}
/**
* @inheritdoc
*/
public function setNode(Node $node) {
#[Override]
public function setNode(Node $node): self {
$this->fileId = null;
$this->nodeType = null;
$this->node = $node;
return $this;
}
/**
* @inheritdoc
*/
public function getNode() {
#[Override]
public function getNode(): Node {
if ($this->node === null) {
if ($this->shareOwner === null || $this->fileId === null) {
throw new NotFoundException();

24
lib/public/Share/IShare.php
View file

@ -7,6 +7,7 @@
*/
namespace OCP\Share;
use OCP\AppFramework\Attribute\Consumable;
use OCP\Files\Cache\ICacheEntry;
use OCP\Files\File;
use OCP\Files\Folder;
@ -21,6 +22,7 @@ use OCP\Share\Exceptions\IllegalIDChangeException;
*
* @since 9.0.0
*/
#[Consumable(since: '9.0.0')]
interface IShare {
/**
* @since 17.0.0
@ -122,62 +124,52 @@ interface IShare {
* It is only allowed to set the internal id of a share once.
* Attempts to override the internal id will result in an IllegalIDChangeException
*
* @param string $id
* @return \OCP\Share\IShare
* @throws IllegalIDChangeException
* @throws \InvalidArgumentException
* @since 9.1.0
*/
public function setId($id);
public function setId(string $id): self;
/**
* Get the internal id of the share.
*
* @return string
* @since 9.0.0
*/
public function getId();
public function getId(): string;
/**
* Get the full share id. This is the <providerid>:<internalid>.
* The full id is unique in the system.
*
* @return string
* @since 9.0.0
* @throws \UnexpectedValueException If the fullId could not be constructed
*/
public function getFullId();
public function getFullId(): string;
/**
* Set the provider id of the share
* It is only allowed to set the provider id of a share once.
* Attempts to override the provider id will result in an IllegalIDChangeException
*
* @param string $id
* @return \OCP\Share\IShare
* @throws IllegalIDChangeException
* @throws \InvalidArgumentException
* @since 9.1.0
*/
public function setProviderId($id);
public function setProviderId(string $id): self;
/**
* Set the node of the file/folder that is shared
*
* @param Node $node
* @return \OCP\Share\IShare The modified object
* @since 9.0.0
*/
public function setNode(Node $node);
public function setNode(Node $node): self;
/**
* Get the node of the file/folder that is shared
*
* @return File|Folder
* @since 9.0.0
* @throws NotFoundException
*/
public function getNode();
public function getNode(): Node;
/**
* Set file id for lazy evaluation of the node

6
tests/lib/Share20/DefaultShareProviderTest.php
View file

@ -463,7 +463,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
]);
$this->assertEquals(1, $qb->executeStatement());
$id = $qb->getLastInsertId();
$id = (string)$qb->getLastInsertId();
$share = $this->createMock(IShare::class);
$share->method('getId')->willReturn($id);
@ -546,7 +546,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
'permissions' => $qb->expr()->literal(13),
]);
$this->assertEquals(1, $qb->executeStatement());
$id = $qb->getLastInsertId();
$id = (string)$qb->getLastInsertId();
$qb = $this->dbConn->getQueryBuilder();
$qb->insert('share')
@ -614,7 +614,7 @@ class DefaultShareProviderTest extends \Test\TestCase {
$qb->executeStatement();
// Get the id
$id = $qb->getLastInsertId();
$id = (string)$qb->getLastInsertId();
$qb = $this->dbConn->getQueryBuilder();
$qb->insert('share')

30
tests/lib/Share20/ManagerTest.php
View file

@ -27,6 +27,7 @@ use OCP\Files\Mount\IMountManager;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\Mount\IShareOwnerlessMount;
use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\Files\Storage\IStorage;
use OCP\HintException;
use OCP\IAppConfig;
@ -778,17 +779,23 @@ class ManagerTest extends \Test\TestCase {
self::invokePrivate($this->manager, 'verifyPassword', ['password']);
}
public function createShare($id, $type, $node, $sharedWith, $sharedBy, $shareOwner,
$permissions, $expireDate = null, $password = null, $attributes = null) {
public function createShare($id, int $type, ?Node $node, $sharedWith, $sharedBy, $shareOwner,
$permissions, $expireDate = null, $password = null, $attributes = null): IShare&MockObject {
$share = $this->createMock(IShare::class);
$share->method('getShareType')->willReturn($type);
$share->method('getSharedWith')->willReturn($sharedWith);
$share->method('getSharedBy')->willReturn($sharedBy);
$share->method('getShareOwner')->willReturn($shareOwner);
$share->method('getNode')->willReturn($node);
if ($node && $node->getId()) {
$share->method('getNodeId')->willReturn($node->getId());
if ($node) {
$share->method('getNode')->willReturn($node);
if ($node->getId()) {
$share->method('getNodeId')->willReturn($node->getId());
}
} else {
$share->method('getNode')->willReturnCallback(function (): never {
throw new NotFoundException();
});
}
$share->method('getPermissions')->willReturn($permissions);
$share->method('getAttributes')->willReturn($attributes);
@ -1046,9 +1053,16 @@ class ManagerTest extends \Test\TestCase {
->method('getId')
->willReturn(42);
// Id 108 is used in the data to refer to the node of the share.
$userFolder->method('getById')
->with(108)
->willReturn([$share->getNode()]);
try {
$node = $share->getNode();
$userFolder->method('getById')
->with(108)
->willReturn([$node]);
} catch (NotFoundException $e) {
$userFolder->method('getById')
->with(108)
->willReturn([]);
}
$userFolder->expects($this->any())
->method('getRelativePath')
->willReturnArgument(0);

29
tests/lib/Share20/ShareTest.php
View file

@ -21,12 +21,9 @@ use PHPUnit\Framework\MockObject\MockObject;
* @package Test\Share20
*/
class ShareTest extends \Test\TestCase {
/** @var IRootFolder|MockObject */
protected $rootFolder;
/** @var IUserManager|MockObject */
protected $userManager;
/** @var IShare */
protected $share;
protected IRootFolder&MockObject $rootFolder;
protected IUserManager&MockObject $userManager;
protected IShare $share;
protected function setUp(): void {
$this->rootFolder = $this->createMock(IRootFolder::class);
@ -34,26 +31,16 @@ class ShareTest extends \Test\TestCase {
$this->share = new Share($this->rootFolder, $this->userManager);
}
public function testSetIdInvalid(): void {
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('String expected.');
$this->share->setId(1.2);
}
public function testSetIdInt(): void {
$this->share->setId(42);
$this->assertEquals('42', $this->share->getId());
}
public function testSetIdString(): void {
$this->share->setId('foo');
$this->assertEquals('foo', $this->share->getId());
}
public function testSetIdOnce(): void {
$this->expectException(IllegalIDChangeException::class);
$this->expectExceptionMessage('Not allowed to assign a new internal id to a share');
@ -62,22 +49,12 @@ class ShareTest extends \Test\TestCase {
$this->share->setId('bar');
}
public function testSetProviderIdInt(): void {
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('String expected.');
$this->share->setProviderId(42);
}
public function testSetProviderIdString(): void {
$this->share->setProviderId('foo');
$this->share->setId('bar');
$this->assertEquals('foo:bar', $this->share->getFullId());
}
public function testSetProviderIdOnce(): void {
$this->expectException(IllegalIDChangeException::class);
$this->expectExceptionMessage('Not allowed to assign a new provider id to a share');