From 7c040c0bf97c2d3a63051a819dc480403d3b96fb Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 7 Jun 2016 12:50:12 +0200 Subject: [PATCH 1/3] Show the path relative to the requesting user A share can only be requested by 3 'types' of people * owner * initiator * recipient So we have to get the path as the current user. Since that is the only path that has any meaning to the user. --- apps/files_sharing/lib/API/Share20OCS.php | 24 +++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php index 90e1f19130..53b27aae0b 100644 --- a/apps/files_sharing/lib/API/Share20OCS.php +++ b/apps/files_sharing/lib/API/Share20OCS.php @@ -100,15 +100,8 @@ class Share20OCS { */ protected function formatShare(\OCP\Share\IShare $share) { $sharedBy = $this->userManager->get($share->getSharedBy()); - // for federated shares the owner can be a remote user, in this - // case we use the initiator - if ($this->userManager->userExists($share->getShareOwner())) { - $shareOwner = $this->userManager->get($share->getShareOwner()); - $localUser = $share->getShareOwner(); - } else { - $shareOwner = $this->userManager->get($share->getSharedBy()); - $localUser = $share->getSharedBy(); - } + $shareOwner = $this->userManager->get($share->getShareOwner()); + $result = [ 'id' => $share->getId(), 'share_type' => $share->getShareType(), @@ -123,8 +116,16 @@ class Share20OCS { 'displayname_file_owner' => $shareOwner !== null ? $shareOwner->getDisplayName() : $share->getShareOwner(), ]; - $node = $share->getNode(); - $result['path'] = $this->rootFolder->getUserFolder($localUser)->getRelativePath($node->getPath()); + $userFolder = $this->rootFolder->getUserFolder($this->currentUser->getUID()); + $nodes = $userFolder->getById($share->getNodeId()); + + if (empty($nodes)) { + throw new NotFoundException(); + } + + $node = $nodes[0]; + + $result['path'] = $userFolder->getRelativePath($node->getPath()); if ($node instanceOf \OCP\Files\Folder) { $result['item_type'] = 'folder'; } else { @@ -536,7 +537,6 @@ class Share20OCS { $shares = array_merge($shares, $federatedShares); } - $formatted = []; foreach ($shares as $share) { try { From 2e2ece753f4474d4ba0edbf22e4984335eec51ff Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 7 Jun 2016 14:17:22 +0200 Subject: [PATCH 2/3] Fix unit tests --- .../tests/API/Share20OCSTest.php | 21 ++++++++++++++++--- apps/files_sharing/tests/ApiTest.php | 9 +++----- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/apps/files_sharing/tests/API/Share20OCSTest.php b/apps/files_sharing/tests/API/Share20OCSTest.php index 02b16d7bf8..b760a0f47a 100644 --- a/apps/files_sharing/tests/API/Share20OCSTest.php +++ b/apps/files_sharing/tests/API/Share20OCSTest.php @@ -433,8 +433,12 @@ class Share20OCSTest extends \Test\TestCase { ->method('getRelativePath') ->will($this->returnArgument(0)); + $userFolder->method('getById') + ->with($share->getNodeId()) + ->willReturn([$share->getNode()]); + $this->rootFolder->method('getUserFolder') - ->with($share->getShareOwner()) + ->with($this->currentUser->getUID()) ->willReturn($userFolder); $this->urlGenerator @@ -2006,8 +2010,19 @@ class Share20OCSTest extends \Test\TestCase { ->willReturn('myLink'); - $this->rootFolder->method('getUserFolder')->with($share->getShareOwner())->will($this->returnSelf()); - $this->rootFolder->method('getRelativePath')->will($this->returnArgument(0)); + $this->rootFolder->method('getUserFolder') + ->with($this->currentUser->getUID()) + ->will($this->returnSelf()); + + if (!$exception) { + $this->rootFolder->method('getById') + ->with($share->getNodeId()) + ->willReturn([$share->getNode()]); + + $this->rootFolder->method('getRelativePath') + ->with($share->getNode()->getPath()) + ->will($this->returnArgument(0)); + } try { $result = $this->invokePrivate($this->ocs, 'formatShare', [$share]); diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index f44c346236..b1a9e0dfbf 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -764,8 +764,7 @@ class ApiTest extends TestCase { // we should get exactly one result $this->assertCount(1, $data); - $expectedPath = $this->folder . $this->subfolder; - $this->assertEquals($expectedPath, $data[0]['path']); + $this->assertEquals($this->subfolder, $data[0]['path']); $this->shareManager->deleteShare($share2); $this->shareManager->deleteShare($share1); @@ -812,8 +811,7 @@ class ApiTest extends TestCase { // we should get exactly one result $this->assertCount(1, $data); - $expectedPath = $this->folder . $this->subfolder . $this->subsubfolder; - $this->assertEquals($expectedPath, $data[0]['path']); + $this->assertEquals($this->subsubfolder, $data[0]['path']); $this->shareManager->deleteShare($share1); $this->shareManager->deleteShare($share2); @@ -922,8 +920,7 @@ class ApiTest extends TestCase { // we should get exactly one result $this->assertCount(1, $data); - $expectedPath = $this->folder.$this->subfolder.$this->filename; - $this->assertEquals($expectedPath, $data[0]['path']); + $this->assertEquals($this->filename, $data[0]['path']); $this->shareManager->deleteShare($share1); $this->shareManager->deleteShare($share2); From 72c5535492f10024f25f4b641c4c481f203a2872 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 7 Jun 2016 14:21:48 +0200 Subject: [PATCH 3/3] Extend unit tests --- apps/files_sharing/tests/ApiTest.php | 34 +++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index b1a9e0dfbf..058b0c4758 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -800,6 +800,9 @@ class ApiTest extends TestCase { ->setPermissions(1); $share3 = $this->shareManager->createShare($share3); + /* + * Test as recipient + */ $request = $this->createRequest(['path' => '/', 'subfiles' => 'true']); $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER3); $result = $ocs->getShares(); @@ -810,9 +813,38 @@ class ApiTest extends TestCase { // we should get exactly one result $this->assertCount(1, $data); - $this->assertEquals($this->subsubfolder, $data[0]['path']); + /* + * Test for first owner/initiator + */ + $request = $this->createRequest([]); + $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER1); + $result = $ocs->getShares(); + $this->assertTrue($result->succeeded()); + + // test should return one share within $this->folder + $data = $result->getData(); + + // we should get exactly one result + $this->assertCount(1, $data); + $this->assertEquals($this->folder . $this->subfolder, $data[0]['path']); + + /* + * Test for second initiator + */ + $request = $this->createRequest([]); + $ocs = $this->createOCS($request, self::TEST_FILES_SHARING_API_USER2); + $result = $ocs->getShares(); + $this->assertTrue($result->succeeded()); + + // test should return one share within $this->folder + $data = $result->getData(); + + // we should get exactly one result + $this->assertCount(1, $data); + $this->assertEquals($this->subfolder . $this->subsubfolder, $data[0]['path']); + $this->shareManager->deleteShare($share1); $this->shareManager->deleteShare($share2); $this->shareManager->deleteShare($share3);