From a2b8483779b5cb868309ca3f98051bfcaafd6ff9 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 23 Nov 2015 17:10:58 +0100 Subject: [PATCH] [Sharing 2.0] Default share provider only generic DI No injection of userfolders etc. Only generic DI components (IRootFolder) etc should be used to make sure we can also run this from the cli --- apps/files_sharing/api/ocssharewrapper.php | 2 +- lib/private/share20/defaultshareprovider.php | 22 +- .../lib/share20/defaultshareprovidertest.php | 252 ++++++++---------- 3 files changed, 120 insertions(+), 156 deletions(-) diff --git a/apps/files_sharing/api/ocssharewrapper.php b/apps/files_sharing/api/ocssharewrapper.php index b93a994f75..ab54e5e5e3 100644 --- a/apps/files_sharing/api/ocssharewrapper.php +++ b/apps/files_sharing/api/ocssharewrapper.php @@ -34,7 +34,7 @@ class OCSShareWrapper { \OC::$server->getDatabaseConnection(), \OC::$server->getUserManager(), \OC::$server->getGroupManager(), - \OC::$server->getUserFolder() + \OC::$server->getRootFolder() ) ), \OC::$server->getGroupManager(), diff --git a/lib/private/share20/defaultshareprovider.php b/lib/private/share20/defaultshareprovider.php index 5805e41d41..15add93ce6 100644 --- a/lib/private/share20/defaultshareprovider.php +++ b/lib/private/share20/defaultshareprovider.php @@ -35,17 +35,17 @@ class DefaultShareProvider implements IShareProvider { /** @var \OCP\IGroupManager */ private $groupManager; - /** @var \OCP\Files\Folder */ - private $userFolder; + /** @var \OCP\Files\IRootFolder */ + private $rootFolder; public function __construct(\OCP\IDBConnection $connection, \OCP\IUserManager $userManager, \OCP\IGroupManager $groupManager, - \OCP\Files\Folder $userFolder) { + \OCP\Files\IRootFolder $rootFolder) { $this->dbConn = $connection; $this->userManager = $userManager; $this->groupManager = $groupManager; - $this->userFolder = $userFolder; + $this->rootFolder = $rootFolder; } /** @@ -218,14 +218,14 @@ class DefaultShareProvider implements IShareProvider { $share->setSharedBy($this->userManager->get($data['uid_owner'])); // TODO: getById can return an array. How to handle this properly?? - $path = $this->userFolder->getById((int)$data['file_source']); - $path = $path[0]; - $share->setPath($path); + $folder = $this->rootFolder->getUserFolder($share->getSharedBy()->getUID()); + $path = $folder->getById((int)$data['file_source'])[0]; - $owner = $path->getStorage()->getOwner('.'); - if ($owner !== false) { - $share->setShareOwner($this->userManager->get($owner)); - } + $owner = $path->getOwner(); + $share->setShareOwner($owner); + + $path = $this->rootFolder->getUserFolder($owner->getUID())->getById((int)$data['file_source'])[0]; + $share->setPath($path); if ($data['expiration'] !== null) { $expiration = \DateTime::createFromFormat('Y-m-d H:i:s', $data['expiration']); diff --git a/tests/lib/share20/defaultshareprovidertest.php b/tests/lib/share20/defaultshareprovidertest.php index 4c0b552394..dc45bc7c08 100644 --- a/tests/lib/share20/defaultshareprovidertest.php +++ b/tests/lib/share20/defaultshareprovidertest.php @@ -23,7 +23,7 @@ namespace Test\Share20; use OCP\IDBConnection; use OCP\IUserManager; use OCP\IGroupManager; -use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OC\Share20\DefaultShareProvider; class DefaultShareProviderTest extends \Test\TestCase { @@ -37,8 +37,8 @@ class DefaultShareProviderTest extends \Test\TestCase { /** @var IGroupManager */ protected $groupManager; - /** @var Folder */ - protected $userFolder; + /** @var IRootFolder */ + protected $rootFolder; /** @var DefaultShareProvider */ protected $provider; @@ -47,7 +47,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->dbConn = \OC::$server->getDatabaseConnection(); $this->userManager = $this->getMock('OCP\IUserManager'); $this->groupManager = $this->getMock('OCP\IGroupManager'); - $this->userFolder = $this->getMock('OCP\Files\Folder'); + $this->rootFolder = $this->getMock('OCP\Files\IRootFolder'); //Empty share table $this->dbConn->getQueryBuilder()->delete('share')->execute(); @@ -56,7 +56,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->dbConn, $this->userManager, $this->groupManager, - $this->userFolder + $this->rootFolder ); } @@ -65,7 +65,7 @@ class DefaultShareProviderTest extends \Test\TestCase { } /** - * @expectedException OC\Share20\Exception\ShareNotFound + * @expectedException \OC\Share20\Exception\ShareNotFound */ public function testGetShareByIdNotExist() { $this->provider->getShareById(1); @@ -97,25 +97,30 @@ class DefaultShareProviderTest extends \Test\TestCase { $id = $id['id']; $cursor->closeCursor(); - $storage = $this->getMock('OC\Files\Storage\Storage'); - $storage - ->expects($this->once()) - ->method('getOwner') - ->willReturn('shareOwner'); - $path = $this->getMock('OCP\Files\File'); - $path - ->expects($this->once()) - ->method('getStorage') - ->wilLReturn($storage); - $this->userFolder - ->expects($this->once()) - ->method('getById') - ->with(42) - ->willReturn([$path]); - $sharedWith = $this->getMock('OCP\IUser'); $sharedBy = $this->getMock('OCP\IUser'); + $sharedBy->method('getUID')->willReturn('sharedBy'); $shareOwner = $this->getMock('OCP\IUser'); + $shareOwner->method('getUID')->willReturn('shareOwner'); + + $sharedByPath = $this->getMock('\OCP\Files\File'); + $ownerPath = $this->getMock('\OCP\Files\File'); + + $sharedByPath->method('getOwner')->willReturn($shareOwner); + + $sharedByFolder = $this->getMock('\OCP\Files\Folder'); + $sharedByFolder->method('getById')->with(42)->willReturn([$sharedByPath]); + + $shareOwnerFolder = $this->getMock('\OCP\Files\Folder'); + $shareOwnerFolder->method('getById')->with(42)->willReturn([$ownerPath]); + + $this->rootFolder + ->method('getUserFolder') + ->will($this->returnValueMap([ + ['sharedBy', $sharedByFolder], + ['shareOwner', $shareOwnerFolder], + ])); + $this->userManager ->method('get') ->will($this->returnValueMap([ @@ -131,7 +136,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals($sharedWith, $share->getSharedWith()); $this->assertEquals($sharedBy, $share->getSharedBy()); $this->assertEquals($shareOwner, $share->getShareOwner()); - $this->assertEquals($path, $share->getPath()); + $this->assertEquals($ownerPath, $share->getPath()); $this->assertEquals(13, $share->getPermissions()); $this->assertEquals(null, $share->getToken()); $this->assertEquals(null, $share->getExpirationDate()); @@ -164,25 +169,30 @@ class DefaultShareProviderTest extends \Test\TestCase { $id = $id['id']; $cursor->closeCursor(); - $storage = $this->getMock('OC\Files\Storage\Storage'); - $storage - ->expects($this->once()) - ->method('getOwner') - ->willReturn('shareOwner'); - $path = $this->getMock('OCP\Files\Folder'); - $path - ->expects($this->once()) - ->method('getStorage') - ->wilLReturn($storage); - $this->userFolder - ->expects($this->once()) - ->method('getById') - ->with(42) - ->willReturn([$path]); - $sharedWith = $this->getMock('OCP\IGroup'); $sharedBy = $this->getMock('OCP\IUser'); + $sharedBy->method('getUID')->willReturn('sharedBy'); $shareOwner = $this->getMock('OCP\IUser'); + $shareOwner->method('getUID')->willReturn('shareOwner'); + + $sharedByPath = $this->getMock('\OCP\Files\Folder'); + $ownerPath = $this->getMock('\OCP\Files\Folder'); + + $sharedByPath->method('getOwner')->willReturn($shareOwner); + + $sharedByFolder = $this->getMock('\OCP\Files\Folder'); + $sharedByFolder->method('getById')->with(42)->willReturn([$sharedByPath]); + + $shareOwnerFolder = $this->getMock('\OCP\Files\Folder'); + $shareOwnerFolder->method('getById')->with(42)->willReturn([$ownerPath]); + + $this->rootFolder + ->method('getUserFolder') + ->will($this->returnValueMap([ + ['sharedBy', $sharedByFolder], + ['shareOwner', $shareOwnerFolder], + ])); + $this->userManager ->method('get') ->will($this->returnValueMap([ @@ -202,7 +212,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals($sharedWith, $share->getSharedWith()); $this->assertEquals($sharedBy, $share->getSharedBy()); $this->assertEquals($shareOwner, $share->getShareOwner()); - $this->assertEquals($path, $share->getPath()); + $this->assertEquals($ownerPath, $share->getPath()); $this->assertEquals(13, $share->getPermissions()); $this->assertEquals(null, $share->getToken()); $this->assertEquals(null, $share->getExpirationDate()); @@ -237,24 +247,29 @@ class DefaultShareProviderTest extends \Test\TestCase { $id = $id['id']; $cursor->closeCursor(); - $storage = $this->getMock('OC\Files\Storage\Storage'); - $storage - ->expects($this->once()) - ->method('getOwner') - ->willReturn('shareOwner'); - $path = $this->getMock('OCP\Files\Node'); - $path - ->expects($this->once()) - ->method('getStorage') - ->wilLReturn($storage); - $this->userFolder - ->expects($this->once()) - ->method('getById') - ->with(42) - ->willReturn([$path]); - $sharedBy = $this->getMock('OCP\IUser'); + $sharedBy->method('getUID')->willReturn('sharedBy'); $shareOwner = $this->getMock('OCP\IUser'); + $shareOwner->method('getUID')->willReturn('shareOwner'); + + $sharedByPath = $this->getMock('\OCP\Files\Folder'); + $ownerPath = $this->getMock('\OCP\Files\Folder'); + + $sharedByPath->method('getOwner')->willReturn($shareOwner); + + $sharedByFolder = $this->getMock('\OCP\Files\Folder'); + $sharedByFolder->method('getById')->with(42)->willReturn([$sharedByPath]); + + $shareOwnerFolder = $this->getMock('\OCP\Files\Folder'); + $shareOwnerFolder->method('getById')->with(42)->willReturn([$ownerPath]); + + $this->rootFolder + ->method('getUserFolder') + ->will($this->returnValueMap([ + ['sharedBy', $sharedByFolder], + ['shareOwner', $shareOwnerFolder], + ])); + $this->userManager ->method('get') ->will($this->returnValueMap([ @@ -269,79 +284,13 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals('sharedWith', $share->getPassword()); $this->assertEquals($sharedBy, $share->getSharedBy()); $this->assertEquals($shareOwner, $share->getShareOwner()); - $this->assertEquals($path, $share->getPath()); + $this->assertEquals($ownerPath, $share->getPath()); $this->assertEquals(13, $share->getPermissions()); $this->assertEquals('token', $share->getToken()); $this->assertEquals(\DateTime::createFromFormat('Y-m-d H:i:s', '2000-01-02 00:00:00'), $share->getExpirationDate()); $this->assertEquals('myTarget', $share->getTarget()); } - public function testGetShareByIdRemoteShare() { - $qb = $this->dbConn->getQueryBuilder(); - - $qb->insert('share') - ->values([ - 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_REMOTE), - 'share_with' => $qb->expr()->literal('sharedWith'), - 'uid_owner' => $qb->expr()->literal('sharedBy'), - 'item_type' => $qb->expr()->literal('file'), - 'file_source' => $qb->expr()->literal(42), - 'file_target' => $qb->expr()->literal('myTarget'), - 'permissions' => $qb->expr()->literal(13), - ]); - $this->assertEquals(1, $qb->execute()); - - // Get the id - $qb = $this->dbConn->getQueryBuilder(); - $cursor = $qb->select('id') - ->from('share') - ->setMaxResults(1) - ->orderBy('id', 'DESC') - ->execute(); - $id = $cursor->fetch(); - $id = $id['id']; - $cursor->closeCursor(); - - - $storage = $this->getMock('OC\Files\Storage\Storage'); - $storage - ->expects($this->once()) - ->method('getOwner') - ->willReturn('shareOwner'); - $path = $this->getMock('OCP\Files\Node'); - $path - ->expects($this->once()) - ->method('getStorage') - ->wilLReturn($storage); - $this->userFolder - ->expects($this->once()) - ->method('getById') - ->with(42) - ->willReturn([$path]); - - $sharedBy = $this->getMock('OCP\IUser'); - $shareOwner = $this->getMock('OCP\IUser'); - $this->userManager - ->method('get') - ->will($this->returnValueMap([ - ['sharedBy', $sharedBy], - ['shareOwner', $shareOwner], - ])); - - $share = $this->provider->getShareById($id); - - $this->assertEquals($id, $share->getId()); - $this->assertEquals(\OCP\Share::SHARE_TYPE_REMOTE, $share->getShareType()); - $this->assertEquals('sharedWith', $share->getSharedWith()); - $this->assertEquals($sharedBy, $share->getSharedBy()); - $this->assertEquals($shareOwner, $share->getShareOwner()); - $this->assertEquals($path, $share->getPath()); - $this->assertEquals(13, $share->getPermissions()); - $this->assertEquals(null, $share->getToken()); - $this->assertEquals(null, $share->getExpirationDate()); - $this->assertEquals('myTarget', $share->getTarget()); - } - public function testDeleteSingleShare() { $qb = $this->dbConn->getQueryBuilder(); $qb->insert('share') @@ -376,7 +325,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->dbConn, $this->userManager, $this->groupManager, - $this->userFolder, + $this->rootFolder, ] ) ->setMethods(['getShareById']) @@ -437,7 +386,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $db, $this->userManager, $this->groupManager, - $this->userFolder, + $this->rootFolder, ] ) ->setMethods(['getShareById']) @@ -504,30 +453,45 @@ class DefaultShareProviderTest extends \Test\TestCase { ]); $qb->execute(); - - $storage = $this->getMock('OC\Files\Storage\Storage'); - $storage - ->method('getOwner') - ->willReturn('shareOwner'); - $path1 = $this->getMock('OCP\Files\File'); - $path1->expects($this->once())->method('getStorage')->willReturn($storage); - $path2 = $this->getMock('OCP\Files\Folder'); - $path2->expects($this->once())->method('getStorage')->willReturn($storage); - $this->userFolder - ->method('getById') - ->will($this->returnValueMap([ - [1, [$path1]], - [3, [$path2]], - ])); - $shareOwner = $this->getMock('OCP\IUser'); + $shareOwner->method('getUID')->willReturn('shareOwner'); $user1 = $this->getMock('OCP\IUser'); $user2 = $this->getMock('OCP\IUser'); + $user2->method('getUID')->willReturn('user2'); $user3 = $this->getMock('OCP\IUser'); + $user3->method('getUID')->willReturn('user3'); + + $user2Path = $this->getMock('\OCP\Files\File'); + $user2Path->expects($this->once())->method('getOwner')->willReturn($shareOwner); + $user2Folder = $this->getMock('\OCP\Files\Folder'); + $user2Folder->expects($this->once()) + ->method('getById') + ->with(1) + ->willReturn([$user2Path]); + + $user3Path = $this->getMock('\OCP\Files\Folder'); + $user3Path->expects($this->once())->method('getOwner')->willReturn($shareOwner); + $user3Folder = $this->getMock('\OCP\Files\Folder'); + $user3Folder->expects($this->once()) + ->method('getById') + ->with(3) + ->willReturn([$user3Path]); + + $ownerPath = $this->getMock('\OCP\Files\Folder'); + $ownerFolder = $this->getMock('\OCP\Files\Folder'); + $ownerFolder->method('getById')->willReturn([$ownerPath]); + + $this->rootFolder + ->method('getUserFolder') + ->will($this->returnValueMap([ + ['shareOwner', $ownerFolder], + ['user2', $user2Folder], + ['user3', $user3Folder], + ])); + $this->userManager ->method('get') ->will($this->returnValueMap([ - ['shareOwner', $shareOwner], ['user1', $user1], ['user2', $user2], ['user3', $user3], @@ -552,7 +516,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals($user1, $children[0]->getSharedWith()); $this->assertEquals($user2, $children[0]->getSharedBy()); $this->assertEquals($shareOwner, $children[0]->getShareOwner()); - $this->assertEquals($path1, $children[0]->getPath()); + $this->assertEquals($ownerPath, $children[0]->getPath()); $this->assertEquals(2, $children[0]->getPermissions()); $this->assertEquals(null, $children[0]->getToken()); $this->assertEquals(null, $children[0]->getExpirationDate()); @@ -563,7 +527,7 @@ class DefaultShareProviderTest extends \Test\TestCase { $this->assertEquals($group1, $children[1]->getSharedWith()); $this->assertEquals($user3, $children[1]->getSharedBy()); $this->assertEquals($shareOwner, $children[1]->getShareOwner()); - $this->assertEquals($path2, $children[1]->getPath()); + $this->assertEquals($ownerPath, $children[1]->getPath()); $this->assertEquals(4, $children[1]->getPermissions()); $this->assertEquals(null, $children[1]->getToken()); $this->assertEquals(null, $children[1]->getExpirationDate());