Merge pull request #2824 from nextcloud/ext-storage-expireversions

Properly expire ext storage versions (#26601)

3 files changed, 29 insertions, 10 deletions

diff --git a/apps/files_versions/lib/Command/Expire.php b/apps/files_versions/lib/Command/Expire.php
index fecffd2f3cc..794cedbac9a 100644
--- a/apps/files_versions/lib/Command/Expire.php
+++ b/apps/files_versions/lib/Command/Expire.php
@@ -58,8 +58,6 @@ class Expire implements ICommand {
 			return;
 		}
 
-		\OC_Util::setupFS($this->user);
-		Storage::expire($this->fileName);
-		\OC_Util::tearDownFS();
+		Storage::expire($this->fileName, $this->user);
 	}
 }
diff --git a/apps/files_versions/lib/Storage.php b/apps/files_versions/lib/Storage.php
index 1aa19274c48..9aa9e6d5260 100644
--- a/apps/files_versions/lib/Storage.php
+++ b/apps/files_versions/lib/Storage.php
@@ -688,30 +688,39 @@ class Storage {
 	}
 
 	/**
-	 * Expire versions which exceed the quota
+	 * Expire versions which exceed the quota.
 	 *
-	 * @param string $filename
+	 * This will setup the filesystem for the given user but will not
+	 * tear it down afterwards.
+	 *
+	 * @param string $filename path to file to expire
+	 * @param string $uid user for which to expire the version
 	 * @return bool|int|null
 	 */
-	public static function expire($filename) {
+	public static function expire($filename, $uid) {
 		$config = \OC::$server->getConfig();
 		$expiration = self::getExpiration();
 
 		if($config->getSystemValue('files_versions', Storage::DEFAULTENABLED)=='true' && $expiration->isEnabled()) {
+			// get available disk space for user
+			$user = \OC::$server->getUserManager()->get($uid);
+			if (is_null($user)) {
+				\OCP\Util::writeLog('files_versions', 'Backends provided no user object for ' . $uid, \OCP\Util::ERROR);
+				throw new \OC\User\NoUserException('Backends provided no user object for ' . $uid);
+			}
+
+			\OC_Util::setupFS($uid);
 
 			if (!Filesystem::file_exists($filename)) {
 				return false;
 			}
 
-			list($uid, $filename) = self::getUidAndFilename($filename);
 			if (empty($filename)) {
 				// file maybe renamed or deleted
 				return false;
 			}
 			$versionsFileview = new View('/'.$uid.'/files_versions');
 
-			// get available disk space for user
-			$user = \OC::$server->getUserManager()->get($uid);
 			$softQuota = true;
 			$quota = $user->getQuota();
 			if ( $quota === null || $quota === 'none' ) {
diff --git a/apps/files_versions/tests/VersioningTest.php b/apps/files_versions/tests/VersioningTest.php
index de79327e084..a1649b2b600 100644
--- a/apps/files_versions/tests/VersioningTest.php
+++ b/apps/files_versions/tests/VersioningTest.php
@@ -612,7 +612,19 @@ class VersioningTest extends \Test\TestCase {
 		// needed to have a FS setup (the background job does this)
 		\OC_Util::setupFS(self::TEST_VERSIONS_USER);
 
-		$this->assertFalse(\OCA\Files_Versions\Storage::expire('/void/unexist.txt'));
+		$this->assertFalse(\OCA\Files_Versions\Storage::expire('/void/unexist.txt', self::TEST_VERSIONS_USER));
+	}
+
+	/**
+	 * @expectedException \OC\User\NoUserException
+	 */
+	public function testExpireNonexistingUser() {
+		$this->logout();
+		// needed to have a FS setup (the background job does this)
+		\OC_Util::setupFS(self::TEST_VERSIONS_USER);
+		\OC\Files\Filesystem::file_put_contents("test.txt", "test file");
+
+		$this->assertFalse(\OCA\Files_Versions\Storage::expire('test.txt', 'unexist'));
 	}
 
 	public function testRestoreSameStorage() {