From b26b8d17ebcd843ddba35bf446f3f5a7655ee38d Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 7 Mar 2016 10:40:49 +0100 Subject: [PATCH 1/2] Remove support for old shares in the default share provider In 9.0 we converted the old shares to the new shares. So for 9.1 we can savely remove the fallback code. This code was required when there was no initiator set. * Fixed unit tests --- lib/private/Share20/DefaultShareProvider.php | 27 +---- .../lib/share20/defaultshareprovidertest.php | 112 ++---------------- 2 files changed, 16 insertions(+), 123 deletions(-) diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index b91ac24d44..b5963e0e3c 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -471,18 +471,7 @@ class DefaultShareProvider implements IShareProvider { * Reshares for this user are shares where they are the owner. */ if ($reshares === false) { - //Special case for old shares created via the web UI - $or1 = $qb->expr()->andX( - $qb->expr()->eq('uid_owner', $qb->createNamedParameter($userId)), - $qb->expr()->isNull('uid_initiator') - ); - - $qb->andWhere( - $qb->expr()->orX( - $qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId)), - $or1 - ) - ); + $qb->andWhere($qb->expr()->eq('uid_initiator', $qb->createNamedParameter($userId))); } else { $qb->andWhere( $qb->expr()->orX( @@ -765,18 +754,8 @@ class DefaultShareProvider implements IShareProvider { $share->setToken($data['token']); } - if ($data['uid_initiator'] === null) { - //OLD SHARE - $share->setSharedBy($data['uid_owner']); - $path = $this->getNode($share->getSharedBy(), (int)$data['file_source']); - - $owner = $path->getOwner(); - $share->setShareOwner($owner->getUID()); - } else { - //New share! - $share->setSharedBy($data['uid_initiator']); - $share->setShareOwner($data['uid_owner']); - } + $share->setSharedBy($data['uid_initiator']); + $share->setShareOwner($data['uid_owner']); $share->setNodeId((int)$data['file_source']); $share->setNodeType($data['item_type']); diff --git a/tests/lib/share20/defaultshareprovidertest.php b/tests/lib/share20/defaultshareprovidertest.php index 6002498968..610de3b623 100644 --- a/tests/lib/share20/defaultshareprovidertest.php +++ b/tests/lib/share20/defaultshareprovidertest.php @@ -123,7 +123,8 @@ class DefaultShareProviderTest extends \Test\TestCase { ->values([ 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_USER), 'share_with' => $qb->expr()->literal('sharedWith'), - 'uid_owner' => $qb->expr()->literal('sharedBy'), + 'uid_owner' => $qb->expr()->literal('shareOwner'), + 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), 'file_source' => $qb->expr()->literal(42), 'file_target' => $qb->expr()->literal('myTarget'), @@ -133,27 +134,18 @@ class DefaultShareProviderTest extends \Test\TestCase { $id = $qb->getLastInsertId(); - $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], ])); @@ -257,7 +249,8 @@ class DefaultShareProviderTest extends \Test\TestCase { ->values([ 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_GROUP), 'share_with' => $qb->expr()->literal('sharedWith'), - 'uid_owner' => $qb->expr()->literal('sharedBy'), + 'uid_owner' => $qb->expr()->literal('shareOwner'), + 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), 'file_source' => $qb->expr()->literal(42), 'file_target' => $qb->expr()->literal('myTarget'), @@ -268,27 +261,13 @@ class DefaultShareProviderTest extends \Test\TestCase { // Get the id $id = $qb->getLastInsertId(); - $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], ])); @@ -351,7 +330,8 @@ class DefaultShareProviderTest extends \Test\TestCase { ->values([ 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_LINK), 'share_with' => $qb->expr()->literal('sharedWith'), - 'uid_owner' => $qb->expr()->literal('sharedBy'), + 'uid_owner' => $qb->expr()->literal('shareOwner'), + 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), 'file_source' => $qb->expr()->literal(42), 'file_target' => $qb->expr()->literal('myTarget'), @@ -363,36 +343,16 @@ class DefaultShareProviderTest extends \Test\TestCase { $id = $qb->getLastInsertId(); - $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([ - ['sharedBy', $sharedBy], - ['shareOwner', $shareOwner], - ])); - $share = $this->provider->getShareById($id); $this->assertEquals($id, $share->getId()); @@ -544,7 +504,8 @@ class DefaultShareProviderTest extends \Test\TestCase { ->values([ 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_USER), 'share_with' => $qb->expr()->literal('sharedWith'), - 'uid_owner' => $qb->expr()->literal('sharedBy'), + 'uid_owner' => $qb->expr()->literal('shareOwner'), + 'uid_initiator' => $qb->expr()->literal('sharedBy'), 'item_type' => $qb->expr()->literal('file'), 'file_source' => $qb->expr()->literal(42), 'file_target' => $qb->expr()->literal('myTarget'), @@ -553,22 +514,15 @@ class DefaultShareProviderTest extends \Test\TestCase { $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(); + $id = $qb->getLastInsertId(); $qb = $this->dbConn->getQueryBuilder(); $qb->insert('share') ->values([ 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_USER), 'share_with' => $qb->expr()->literal('user1'), - 'uid_owner' => $qb->expr()->literal('user2'), + 'uid_owner' => $qb->expr()->literal('shareOwner'), + 'uid_initiator' => $qb->expr()->literal('user2'), 'item_type' => $qb->expr()->literal('file'), 'file_source' => $qb->expr()->literal(1), 'file_target' => $qb->expr()->literal('myTarget1'), @@ -582,7 +536,8 @@ class DefaultShareProviderTest extends \Test\TestCase { ->values([ 'share_type' => $qb->expr()->literal(\OCP\Share::SHARE_TYPE_GROUP), 'share_with' => $qb->expr()->literal('group1'), - 'uid_owner' => $qb->expr()->literal('user3'), + 'uid_owner' => $qb->expr()->literal('shareOwner'), + 'uid_initiator' => $qb->expr()->literal('user3'), 'item_type' => $qb->expr()->literal('folder'), 'file_source' => $qb->expr()->literal(3), 'file_target' => $qb->expr()->literal('myTarget2'), @@ -591,30 +546,6 @@ class DefaultShareProviderTest extends \Test\TestCase { ]); $qb->execute(); - $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]); @@ -623,23 +554,6 @@ class DefaultShareProviderTest extends \Test\TestCase { ->method('getUserFolder') ->will($this->returnValueMap([ ['shareOwner', $ownerFolder], - ['user2', $user2Folder], - ['user3', $user3Folder], - ])); - - $this->userManager - ->method('get') - ->will($this->returnValueMap([ - ['user1', $user1], - ['user2', $user2], - ['user3', $user3], - ])); - - $group1 = $this->getMock('OCP\IGroup'); - $this->groupManager - ->method('get') - ->will($this->returnValueMap([ - ['group1', $group1] ])); $share = $this->getMock('\OCP\Share\IShare'); From da1dbb52e42fd1acb398a0f223e1630d777a8c3c Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 7 Mar 2016 12:27:31 +0100 Subject: [PATCH 2/2] Remove dead function This was used when we did not have lazy shares yet. Now that we no longer support legacy shares this can go. --- lib/private/Share20/DefaultShareProvider.php | 24 -------------------- 1 file changed, 24 deletions(-) diff --git a/lib/private/Share20/DefaultShareProvider.php b/lib/private/Share20/DefaultShareProvider.php index b5963e0e3c..f6171f8799 100644 --- a/lib/private/Share20/DefaultShareProvider.php +++ b/lib/private/Share20/DefaultShareProvider.php @@ -770,30 +770,6 @@ class DefaultShareProvider implements IShareProvider { return $share; } - /** - * Get the node with file $id for $user - * - * @param string $user The userId - * @param int $id - * @return \OCP\Files\File|\OCP\Files\Folder - * @throws InvalidShare - */ - private function getNode($user, $id) { - try { - $userFolder = $this->rootFolder->getUserFolder($user); - } catch (NotFoundException $e) { - throw new InvalidShare(); - } - - $nodes = $userFolder->getById($id); - - if (empty($nodes)) { - throw new InvalidShare(); - } - - return $nodes[0]; - } - /** * Resolve a group share to a user specific share * Thus if the user moved their group share make sure this is properly reflected here.