Deduplicate queued trashbin expire jobs

* fixes #20425
* this removes the argument trashbin size from the expire job - it is now
  calculated in the expire job
* the queue now detects properly that the job is already queue and doesn't
  add it again

diff --git a/apps/files_trashbin/command/expire.php b/apps/files_trashbin/command/expire.php
index f34c63b718bd059f60e179f5b5a0e7b28c7b92cc..cb4e803ddc551c4d6ca6aaa67271a60eb3fa89bc 100644 (file)
--- a/apps/files_trashbin/command/expire.php
+++ b/apps/files_trashbin/command/expire.php
@@ -36,18 +36,11 @@ class Expire implements ICommand {
         */
        private $user;
 
-       /**
-        * @var int
-        */
-       private $trashBinSize;
-
        /**
         * @param string $user
-        * @param int $trashBinSize
         */
-       function __construct($user, $trashBinSize) {
+       function __construct($user) {
                $this->user = $user;
-               $this->trashBinSize = $trashBinSize;
        }
 
        public function handle() {
@@ -59,7 +52,7 @@ class Expire implements ICommand {
 
                \OC_Util::tearDownFS();
                \OC_Util::setupFS($this->user);
-               Trashbin::expire($this->trashBinSize, $this->user);
+               Trashbin::expire($this->user);
                \OC_Util::tearDownFS();
        }
 }

diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php
index 94975530799f873d90ca51a9e3ce8cb3d619f9d8..d492810b95f05febbb2fdb340942c72ace1a6608 100644 (file)
--- a/apps/files_trashbin/lib/trashbin.php
+++ b/apps/files_trashbin/lib/trashbin.php
@@ -186,7 +186,6 @@ class Trashbin {
                // get the user for which the filesystem is setup
                $root = Filesystem::getRoot();
                list(, $user) = explode('/', $root);
-               $size = 0;
                list($owner, $ownerPath) = self::getUidAndFilename($file_path);
 
                $ownerView = new \OC\Files\View('/' . $owner);
@@ -207,8 +206,6 @@ class Trashbin {
                $location = $path_parts['dirname'];
                $timestamp = time();
 
-               $userTrashSize = self::getTrashbinSize($user);
-
                // disable proxy to prevent recursive calls
                $trashPath = '/files_trashbin/files/' . $filename . '.d' . $timestamp;
 
@@ -238,7 +235,6 @@ class Trashbin {
                $trashStorage->getUpdater()->renameFromStorage($sourceStorage, $sourceInternalPath, $trashInternalPath);
 
                if ($sizeOfAddedFiles !== false) {
-                       $size = $sizeOfAddedFiles;
                        $query = \OC_DB::prepare("INSERT INTO `*PREFIX*files_trash` (`id`,`timestamp`,`location`,`user`) VALUES (?,?,?,?)");
                        $result = $query->execute(array($filename, $timestamp, $location, $owner));
                        if (!$result) {
@@ -247,7 +243,7 @@ class Trashbin {
                        \OCP\Util::emitHook('\OCA\Files_Trashbin\Trashbin', 'post_moveToTrash', array('filePath' => \OC\Files\Filesystem::normalizePath($file_path),
                                'trashPath' => \OC\Files\Filesystem::normalizePath($filename . '.d' . $timestamp)));
 
-                       $size += self::retainVersions($filename, $owner, $ownerPath, $timestamp);
+                       self::retainVersions($filename, $owner, $ownerPath, $timestamp);
 
                        // if owner !== user we need to also add a copy to the owners trash
                        if ($user !== $owner) {
@@ -255,14 +251,11 @@ class Trashbin {
                        }
                }
 
-               $userTrashSize += $size;
-               self::scheduleExpire($userTrashSize, $user);
+               self::scheduleExpire($user);
 
                // if owner !== user we also need to update the owners trash size
                if ($owner !== $user) {
-                       $ownerTrashSize = self::getTrashbinSize($owner);
-                       $ownerTrashSize += $size;
-                       self::scheduleExpire($ownerTrashSize, $owner);
+                       self::scheduleExpire($owner);
                }
 
                return ($sizeOfAddedFiles === false) ? false : true;
@@ -628,17 +621,17 @@ class Trashbin {
                $freeSpace = self::calculateFreeSpace($size, $user);
 
                if ($freeSpace < 0) {
-                       self::scheduleExpire($size, $user);
+                       self::scheduleExpire($user);
                }
        }
 
        /**
         * clean up the trash bin
         *
-        * @param int $trashBinSize current size of the trash bin
         * @param string $user
         */
-       public static function expire($trashBinSize, $user) {
+       public static function expire($user) {
+               $trashBinSize = self::getTrashbinSize($user);
                $availableSpace = self::calculateFreeSpace($trashBinSize, $user);
                $size = 0;
 
@@ -654,15 +647,15 @@ class Trashbin {
                $size += self::deleteFiles(array_slice($dirContent, $count), $user, $availableSpace);
        }
 
-       /**@param int $trashBinSize current size of the trash bin
+       /**
         * @param string $user
         */
-       private static function scheduleExpire($trashBinSize, $user) {
+       private static function scheduleExpire($user) {
                // let the admin disable auto expire
                $application = new Application();
                $expiration = $application->getContainer()->query('Expiration');
                if ($expiration->isEnabled()) {
-                       \OC::$server->getCommandBus()->push(new Expire($user, $trashBinSize));
+                       \OC::$server->getCommandBus()->push(new Expire($user));
                }
        }
 

diff --git a/apps/files_trashbin/tests/command/expiretest.php b/apps/files_trashbin/tests/command/expiretest.php
index a6a8a6d53a8fd254e71efa3fd6ce4f38c415b317..0d457db280743872c821c8eb8927c7f471734df3 100644 (file)
--- a/apps/files_trashbin/tests/command/expiretest.php
+++ b/apps/files_trashbin/tests/command/expiretest.php
@@ -26,7 +26,7 @@ use Test\TestCase;
 
 class ExpireTest extends TestCase {
        public function testExpireNonExistingUser() {
-               $command = new Expire('test', 0);
+               $command = new Expire('test');
                $command->handle();
 
                $this->assertTrue(true);