getSharesBy should also expire link shares

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;
 	}
 
 	/**

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);

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');