From 8486d617645a5ef75da34df10ed663ef77bed257 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 5 Feb 2016 20:00:07 +0100 Subject: [PATCH] getSharesBy should also expire link shares --- lib/private/share20/manager.php | 48 ++++++++++++- lib/public/share/ishareprovider.php | 2 +- tests/lib/share20/managertest.php | 103 ++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+), 2 deletions(-) diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php index c4fec31fb8..7d0017bf6f 100644 --- a/lib/private/share20/manager.php +++ b/lib/private/share20/manager.php @@ -778,7 +778,53 @@ class Manager implements IManager { $provider = $this->factory->getProviderForType($shareType); - return $provider->getSharesBy($userId, $shareType, $path, $reshares, $limit, $offset); + $shares = $provider->getSharesBy($userId, $shareType, $path, $reshares, $limit, $offset); + + /* + * Work around so we don't return expired shares but still follow + * proper pagination. + */ + if ($shareType === \OCP\Share::SHARE_TYPE_LINK) { + $shares2 = []; + $today = new \DateTime(); + + while(true) { + $added = 0; + foreach ($shares as $share) { + // Check if the share is expired and if so delete it + if ($share->getExpirationDate() !== null && + $share->getExpirationDate() <= $today + ) { + $this->deleteShare($share); + continue; + } + $added++; + $shares2[] = $share; + + if (count($shares2) === $limit) { + break; + } + } + + if (count($shares2) === $limit) { + break; + } + + $offset += $added; + + // Fetch again $limit shares + $shares = $provider->getSharesBy($userId, $shareType, $path, $reshares, $limit, $offset); + + // No more shares means we are done + if (empty($shares)) { + break; + } + } + + $shares = $shares2; + } + + return $shares; } /** diff --git a/lib/public/share/ishareprovider.php b/lib/public/share/ishareprovider.php index 25fa76369a..b339ce63d3 100644 --- a/lib/public/share/ishareprovider.php +++ b/lib/public/share/ishareprovider.php @@ -101,7 +101,7 @@ interface IShareProvider { * @param bool $reshares Also get the shares where $user is the owner instead of just the shares where $user is the initiator * @param int $limit The maximum number of shares to be returned, -1 for all shares * @param int $offset - * @return \OCP\Share\IShare Share[] + * @return \OCP\Share\IShare[] * @since 9.0.0 */ public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offset); diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php index 55c13f03f8..ea0fb1838d 100644 --- a/tests/lib/share20/managertest.php +++ b/tests/lib/share20/managertest.php @@ -1700,6 +1700,109 @@ class ManagerTest extends \Test\TestCase { $manager->createShare($share); } + public function testGetSharesBy() { + $share = $this->manager->newShare(); + + $node = $this->getMock('OCP\Files\Folder'); + + $this->defaultProvider->expects($this->once()) + ->method('getSharesBy') + ->with( + $this->equalTo('user'), + $this->equalTo(\OCP\Share::SHARE_TYPE_USER), + $this->equalTo($node), + $this->equalTo(true), + $this->equalTo(1), + $this->equalTo(1) + )->willReturn([$share]); + + $shares = $this->manager->getSharesBy('user', \OCP\Share::SHARE_TYPE_USER, $node, true, 1, 1); + + $this->assertCount(1, $shares); + $this->assertSame($share, $shares[0]); + } + + /** + * Test to ensure we correctly remove expired link shares + * + * We have 8 Shares and we want the 3 first valid shares. + * share 3-6 and 8 are expired. Thus at the end of this test we should + * have received share 1,2 and 7. And from the manager. Share 3-6 should be + * deleted (as they are evaluated). but share 8 should still be there. + */ + public function testGetSharesByExpiredLinkShares() { + $manager = $this->createManagerMock() + ->setMethods(['deleteShare']) + ->getMock(); + + /** @var \OCP\Share\IShare[] $shares */ + $shares = []; + + /* + * This results in an array of 8 IShare elements + */ + for ($i = 0; $i < 8; $i++) { + $share = $this->manager->newShare(); + $share->setId($i); + $shares[] = $share; + } + + $today = new \DateTime(); + $today->setTime(0,0,0); + + /* + * Set the expiration date to today for some shares + */ + $shares[2]->setExpirationDate($today); + $shares[3]->setExpirationDate($today); + $shares[4]->setExpirationDate($today); + $shares[5]->setExpirationDate($today); + + /** @var \OCP\Share\IShare[] $i */ + $shares2 = []; + for ($i = 0; $i < 8; $i++) { + $shares2[] = clone $shares[$i]; + } + + $node = $this->getMock('OCP\Files\File'); + + /* + * Simulate the getSharesBy call. + */ + $this->defaultProvider + ->method('getSharesBy') + ->will($this->returnCallback(function($uid, $type, $node, $reshares, $limit, $offset) use (&$shares2) { + return array_slice($shares2, $offset, $limit); + })); + + /* + * Simulate the deleteShare call. + */ + $manager->method('deleteShare') + ->will($this->returnCallback(function($share) use (&$shares2) { + for($i = 0; $i < count($shares2); $i++) { + if ($shares2[$i]->getId() === $share->getId()) { + array_splice($shares2, $i, 1); + break; + } + } + })); + + $res = $manager->getSharesBy('user', \OCP\Share::SHARE_TYPE_LINK, $node, true, 3, 0); + + $this->assertCount(3, $res); + $this->assertEquals($shares[0]->getId(), $res[0]->getId()); + $this->assertEquals($shares[1]->getId(), $res[1]->getId()); + $this->assertEquals($shares[6]->getId(), $res[2]->getId()); + + $this->assertCount(4, $shares2); + $this->assertEquals(0, $shares2[0]->getId()); + $this->assertEquals(1, $shares2[1]->getId()); + $this->assertEquals(6, $shares2[2]->getId()); + $this->assertEquals(7, $shares2[3]->getId()); + $this->assertSame($today, $shares[3]->getExpirationDate()); + } + public function testGetShareByToken() { $factory = $this->getMock('\OCP\Share\IProviderFactory');