Properly expire ext storage versions (#26601)

* Properly expire ext storage versions

System-wide external storages have no real owner so the current user is
used as owner. However when running cron.php there is no current user,
so no expiry can be done.

This fix adds an user argument to the expire() function to tell for
which user to expire files. This information is anyway always available
now through the expire command job.

* Move version expire setupFS into the expire function

* Add comment about not tearing down in version Storage::expire()

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

diff --git a/apps/files_versions/lib/Command/Expire.php b/apps/files_versions/lib/Command/Expire.php
index fecffd2f3cc95c97b0bef0201be7e8b818fdcca0..794cedbac9a00161da0b042c72c76b1d224fdef3 100644 (file)
--- 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 1aa19274c48cacdee04777715dca08c94165c0c9..9aa9e6d52605e550ac9beea7de0a3f8147d14444 100644 (file)
--- 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 de79327e084fd3986b1e0543b6f8bfa57a89f748..a1649b2b600d048c5b5a3e1e6666054fa67ab087 100644 (file)
--- 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() {