Make shares expire at the end of the day rather than at the start

lib/private/Share20/Manager.php

@@ -168,6 +168,17 @@ private function splitFullId($id) {
 		return \explode(':', $id, 2);
 	}
 
+	/**
+	 * Decide if a share has expired
+	 *
+	 * @param \OCP\Share\IShare $share
+	 * @return bool
+	 */
+	private function shareHasExpired($share) {
+		$expirationDate = $share->getExpirationDate();
+		return ($expirationDate !== null) && ($expirationDate < new \DateTime("today"));
+	}
+
 	/**
 	 * Verify if a password meets all requirements
 	 *
@@ -1186,14 +1197,13 @@ public function getAllSharesBy($userId, $shareTypes, $nodeIDs, $reshares = false
 
 		$providerIdMap = $this->shareTypeToProviderMap($shareTypes);
 
-		$today = new \DateTime();
 		foreach ($providerIdMap as $providerId => $shareTypeArray) {
 			// Get provider from cache
 			$provider = $this->factory->getProvider($providerId);
 
 			$queriedShares = $provider->getAllSharesBy($userId, $shareTypeArray, $nodeIDs, $reshares);
 			foreach ($queriedShares as $queriedShare) {
-				if ($queriedShare->getExpirationDate() !== null && $queriedShare->getExpirationDate() <= $today) {
+				if ($this->shareHasExpired($queriedShare)) {
 					try {
 						$this->deleteShare($queriedShare);
 					} catch (NotFoundException $e) {
@@ -1227,13 +1237,12 @@ public function getSharesBy($userId, $shareType, $path = null, $reshares = false
 		 * proper pagination.
 		 */
 		$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) {
+				if ($this->shareHasExpired($share)) {
 					try {
 						$this->deleteShare($share);
 					} catch (NotFoundException $e) {
@@ -1287,13 +1296,12 @@ public function getSharedWith($userId, $shareType, $node = null, $limit = 50, $o
 		 * proper pagination.
 		 */
 		$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) {
+				if ($this->shareHasExpired($share)) {
 					try {
 						$this->deleteShare($share);
 					} catch (NotFoundException $e) {
@@ -1343,15 +1351,14 @@ public function getAllSharedWith($userId, $shareTypes, $node = null) {
 		// Aggregate all required $shareTypes by mapping provider to supported shareTypes
 		$providerIdMap = $this->shareTypeToProviderMap($shareTypes);
 
-		$today = new \DateTime();
 		foreach ($providerIdMap as $providerId => $shareTypeArray) {
 			// Get provider from cache
 			$provider = $this->factory->getProvider($providerId);
 
 			// Obtain all shares for all the supported provider types
 			$queriedShares = $provider->getAllSharedWith($userId, $node);
 			foreach ($queriedShares as $queriedShare) {
-				if ($queriedShare->getExpirationDate() !== null && $queriedShare->getExpirationDate() <= $today) {
+				if ($this->shareHasExpired($queriedShare)) {
 					try {
 						$this->deleteShare($queriedShare);
 					} catch (NotFoundException $e) {
@@ -1380,7 +1387,7 @@ public function getShareById($id, $recipient = null) {
 		$share = $provider->getShareById($id, $recipient);
 
 		// Validate link shares expiration date
-		if ($share->getExpirationDate() !== null && $share->getExpirationDate() <= new \DateTime()) {
+		if ($this->shareHasExpired($share)) {
 			$this->deleteShare($share);
 			throw new ShareNotFound();
 		}
@@ -1438,7 +1445,7 @@ public function getShareByToken($token) {
 			$share = $provider->getShareByToken($token);
 		}
 
-		if ($share->getExpirationDate() !== null && $share->getExpirationDate() <= new \DateTime()) {
+		if ($this->shareHasExpired($share)) {
 			$this->deleteShare($share);
 			throw new ShareNotFound();
 		}

tests/acceptance/features/apiShareManagementBasic/createShare.feature

@@ -2304,7 +2304,8 @@ Feature: sharing
       | permissions | read,share    |
     Then the fields of the last response should include
       | expiration | today |
-    And user "user1" should not have any received shares
+    And the response when user "user1" gets the info of the last share should include
+      | expiration | today |
     Examples:
       | ocs_api_version |
       | 1               |

tests/lib/Share20/ManagerTest.php

@@ -597,7 +597,7 @@ public function testGetExpiredShareById() {
 			->setMethods(['deleteShare'])
 			->getMock();
 
-		$date = new \DateTime();
+		$date = new \DateTime("yesterday");
 		$date->setTime(0, 0, 0);
 
 		$share = $this->manager->newShare();
@@ -2763,8 +2763,8 @@ public function testGetAllSharesByExpiration() {
 		$shareExpired = $this->createMock(IShare::class);
 		$shareExpired->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK);
 		$shareExpired->method('getNodeId')->willReturn(201);
-		$today = new \DateTime();
-		$shareExpired->method('getExpirationDate')->willReturn($today);
+		$yesterday = new \DateTime("yesterday");
+		$shareExpired->method('getExpirationDate')->willReturn($yesterday);
 
 		$node = $this->createMock('OCP\Files\Folder');
 		$node->expects($this->any())
@@ -2863,16 +2863,16 @@ public function testGetSharesByExpiredLinkShares() {
 			$shares[] = $share;
 		}
 
-		$today = new \DateTime();
-		$today->setTime(0, 0, 0);
+		$yesterday = new \DateTime("yesterday");
+		$yesterday->setTime(0, 0, 0);
 
 		/*
-		 * Set the expiration date to today for some shares
+		 * Set the expiration date to yesterday for some shares
 		 */
-		$shares[2]->setExpirationDate($today);
-		$shares[3]->setExpirationDate($today);
-		$shares[4]->setExpirationDate($today);
-		$shares[5]->setExpirationDate($today);
+		$shares[2]->setExpirationDate($yesterday);
+		$shares[3]->setExpirationDate($yesterday);
+		$shares[4]->setExpirationDate($yesterday);
+		$shares[5]->setExpirationDate($yesterday);
 
 		/** @var \OCP\Share\IShare[] $i */
 		$shares2 = [];
@@ -2916,7 +2916,7 @@ public function testGetSharesByExpiredLinkShares() {
 		$this->assertEquals(1, $shares2[1]->getId());
 		$this->assertEquals(6, $shares2[2]->getId());
 		$this->assertEquals(7, $shares2[3]->getId());
-		$this->assertSame($today, $shares[3]->getExpirationDate());
+		$this->assertSame($yesterday, $shares[3]->getExpirationDate());
 	}
 
 	public function testGetShareByToken() {
@@ -3006,7 +3006,7 @@ public function testGetShareByTokenExpired() {
 			->setMethods(['deleteShare'])
 			->getMock();
 
-		$date = new \DateTime();
+		$date = new \DateTime("yesterday");
 		$date->setTime(0, 0, 0);
 		$share = $this->manager->newShare();
 		$share->setExpirationDate($date);