From 8ac3d5e36c7d628f1ba4d27c68d54786eed972b2 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Tue, 11 Aug 2015 22:21:32 +0300 Subject: [PATCH 1/5] Add expiration cronjob --- apps/files_trashbin/appinfo/app.php | 1 + apps/files_trashbin/appinfo/install.php | 23 ++++ apps/files_trashbin/appinfo/update.php | 3 + .../lib/backgroundjob/expiretrash.php | 104 ++++++++++++++++++ apps/files_trashbin/lib/expiration.php | 9 ++ apps/files_trashbin/lib/trashbin.php | 2 +- .../tests/backgroundjob/expiretrash.php | 37 +++++++ apps/files_trashbin/tests/expiration.php | 34 ++++++ 8 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 apps/files_trashbin/appinfo/install.php create mode 100644 apps/files_trashbin/lib/backgroundjob/expiretrash.php create mode 100644 apps/files_trashbin/tests/backgroundjob/expiretrash.php diff --git a/apps/files_trashbin/appinfo/app.php b/apps/files_trashbin/appinfo/app.php index 8f079fe6120..4805f9eeafd 100644 --- a/apps/files_trashbin/appinfo/app.php +++ b/apps/files_trashbin/appinfo/app.php @@ -23,6 +23,7 @@ * along with this program. If not, see * */ + $l = \OC::$server->getL10N('files_trashbin'); // register hooks diff --git a/apps/files_trashbin/appinfo/install.php b/apps/files_trashbin/appinfo/install.php new file mode 100644 index 00000000000..dc4c2847c22 --- /dev/null +++ b/apps/files_trashbin/appinfo/install.php @@ -0,0 +1,23 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + + // Cron job for deleting expired trash items +\OC::$server->getJobList()->add('OCA\Files_Trashbin\BackgroundJob\ExpireTrash'); diff --git a/apps/files_trashbin/appinfo/update.php b/apps/files_trashbin/appinfo/update.php index b77210ae4c0..ae018a9da5d 100644 --- a/apps/files_trashbin/appinfo/update.php +++ b/apps/files_trashbin/appinfo/update.php @@ -46,3 +46,6 @@ if (version_compare($installedVersion, '0.6.4', '<')) { $config->setSystemValue('trashbin_retention_obligation', $newObligation); $config->deleteSystemValue('trashbin_auto_expire'); } + +// Cron job for deleting expired trash items +\OC::$server->getJobList()->add('OCA\Files_Trashbin\BackgroundJob\ExpireTrash'); diff --git a/apps/files_trashbin/lib/backgroundjob/expiretrash.php b/apps/files_trashbin/lib/backgroundjob/expiretrash.php new file mode 100644 index 00000000000..350628444cf --- /dev/null +++ b/apps/files_trashbin/lib/backgroundjob/expiretrash.php @@ -0,0 +1,104 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCA\Files_Trashbin\BackgroundJob; + +use OCP\IDBConnection; +use OCP\IUserManager; +use OCA\Files_Trashbin\AppInfo\Application; +use OCA\Files_Trashbin\Expiration; +use OCA\Files_Trashbin\Helper; +use OCA\Files_Trashbin\Trashbin; + +class ExpireTrash extends \OC\BackgroundJob\TimedJob { + + const ITEMS_PER_SESSION = 1000; + + /** + * @var Expiration + */ + private $expiration; + + /** + * @var IDBConnection + */ + private $dbConnection; + + /** + * @var IUserManager + */ + private $userManager; + + public function __construct(IDBConnection $dbConnection = null, IUserManager $userManager = null, Expiration $expiration = null) { + // Run once per 30 minutes + $this->setInterval(60 * 30); + + if (is_null($expiration) || is_null($userManager) || is_null($dbConnection)) { + $this->fixDIForJobs(); + } else { + $this->dbConnection = $dbConnection; + $this->userManager = $userManager; + $this->expiration = $expiration; + } + } + + protected function fixDIForJobs() { + $application = new Application(); + $this->dbConnection = \OC::$server->getDatabaseConnection(); + $this->userManager = \OC::$server->getUserManager(); + $this->expiration = $application->getContainer()->query('Expiration'); + } + + protected function run($argument) { + $maxAge = $this->expiration->getMaxAgeAsTimestamp(); + if (!$maxAge) { + return; + } + $users = $this->userManager->search(''); + foreach ($users as $user) { + $uid = $user->getUID(); + if (!$this->setupFS($uid)) { + continue; + } + $dirContent = Helper::getTrashFiles('/', $uid, 'mtime'); + var_dump($dirContent); + Trashbin::deleteExpiredFiles($dirContent, $uid); + } + + \OC_Util::tearDownFS(); + } + + /** + * Act on behalf on trash item owner + * @param string $user + * @return boolean + */ + private function setupFS($user){ + if (!$this->userManager->userExists($user)) { + return false; + } + + \OC_Util::tearDownFS(); + \OC_Util::setupFS($user); + + return true; + } +} diff --git a/apps/files_trashbin/lib/expiration.php b/apps/files_trashbin/lib/expiration.php index 138540febf8..c8a6abb627b 100644 --- a/apps/files_trashbin/lib/expiration.php +++ b/apps/files_trashbin/lib/expiration.php @@ -105,6 +105,15 @@ class Expiration { return $isOlderThanMax || $isMinReached; } + public function getMaxAgeAsTimestamp(){ + $maxAge = false; + if ($this->isEnabled() && $this->maxAge !== self::NO_OBLIGATION) { + $time = $this->timeFactory->getTime(); + $maxAge = $time - ($this->maxAge * 86400); + } + return $maxAge; + } + private function parseRetentionObligation(){ $splitValues = explode(',', $this->retentionObligation); if (!isset($splitValues[0])) { diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php index 2719eece2a8..784a8e45b21 100644 --- a/apps/files_trashbin/lib/trashbin.php +++ b/apps/files_trashbin/lib/trashbin.php @@ -689,7 +689,7 @@ class Trashbin { * @param string $user * @return array size of deleted files and number of deleted files */ - protected static function deleteExpiredFiles($files, $user) { + public static function deleteExpiredFiles($files, $user) { $application = new Application(); $expiration = $application->getContainer()->query('Expiration'); $size = 0; diff --git a/apps/files_trashbin/tests/backgroundjob/expiretrash.php b/apps/files_trashbin/tests/backgroundjob/expiretrash.php new file mode 100644 index 00000000000..f9f939c40ea --- /dev/null +++ b/apps/files_trashbin/tests/backgroundjob/expiretrash.php @@ -0,0 +1,37 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +use OCA\Files_Trashbin\BackgroundJob\ExpireTrash; + +class ExpireTrash_Test extends \PHPUnit_Framework_TestCase { + public function testConstructAndRun() { + $backgroundJob = new OCA\Files_Trashbin\BackgroundJob\ExpireTrash( + $this->getMock('OCP\IDBConnection'), + $this->getMockBuilder('OCA\Files_Trashbin\Expiration')->disableOriginalConstructor()->getMock() + ); + + $jobList = $this->getMock('\OCP\BackgroundJob\IJobList'); + + /** @var \OC\BackgroundJob\JobList $jobList */ + $backgroundJob->execute($jobList); + $this->assertTrue(true); + } +} diff --git a/apps/files_trashbin/tests/expiration.php b/apps/files_trashbin/tests/expiration.php index 7bd51dccddd..b3c6fcd95af 100644 --- a/apps/files_trashbin/tests/expiration.php +++ b/apps/files_trashbin/tests/expiration.php @@ -24,6 +24,8 @@ use \OCA\Files_Trashbin\Expiration; class Expiration_Test extends \PHPUnit_Framework_TestCase { const SECONDS_PER_DAY = 86400; //60*60*24 + const FAKE_TIME_NOW = 1000000; + public function expirationData(){ $today = 100*self::SECONDS_PER_DAY; $back10Days = (100-10)*self::SECONDS_PER_DAY; @@ -142,6 +144,38 @@ class Expiration_Test extends \PHPUnit_Framework_TestCase { $this->assertAttributeEquals($expectedCanPurgeToSaveSpace, 'canPurgeToSaveSpace', $expiration); } + + public function timestampTestData(){ + return [ + [ 'disabled', false], + [ 'auto', false ], + [ 'auto,auto', false ], + [ 'auto, auto', false ], + [ 'auto, 3', self::FAKE_TIME_NOW - (3*self::SECONDS_PER_DAY) ], + [ '5, auto', false ], + [ '3, 5', self::FAKE_TIME_NOW - (5*self::SECONDS_PER_DAY) ], + [ '10, 3', self::FAKE_TIME_NOW - (10*self::SECONDS_PER_DAY) ], + ]; + } + + + /** + * @dataProvider timestampTestData + * + * @param string $configValue + * @param int $expectedMaxAgeTimestamp + */ + public function testGetMaxAgeAsTimestamp($configValue, $expectedMaxAgeTimestamp){ + $mockedConfig = $this->getMockedConfig($configValue); + $mockedTimeFactory = $this->getMockedTimeFactory( + self::FAKE_TIME_NOW + ); + + $expiration = new Expiration($mockedConfig, $mockedTimeFactory); + $actualTimestamp = $expiration->getMaxAgeAsTimestamp(); + $this->assertEquals($expectedMaxAgeTimestamp, $actualTimestamp); + } + /** * * @param int $time From 867ed67aa501f11f363a44a40c3b046bbaf08917 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Wed, 9 Sep 2015 23:29:52 +0300 Subject: [PATCH 2/5] Fix tests --- apps/files_trashbin/tests/backgroundjob/expiretrash.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/files_trashbin/tests/backgroundjob/expiretrash.php b/apps/files_trashbin/tests/backgroundjob/expiretrash.php index f9f939c40ea..f39714f8a30 100644 --- a/apps/files_trashbin/tests/backgroundjob/expiretrash.php +++ b/apps/files_trashbin/tests/backgroundjob/expiretrash.php @@ -19,12 +19,15 @@ * */ -use OCA\Files_Trashbin\BackgroundJob\ExpireTrash; +namespace OCA\Files_Trashbin\Tests\BackgroundJob\ExpireTrash; + +use \OCA\Files_Trashbin\BackgroundJob\ExpireTrash; -class ExpireTrash_Test extends \PHPUnit_Framework_TestCase { +class ExpireTrash_Test extends \Test\TestCase { public function testConstructAndRun() { - $backgroundJob = new OCA\Files_Trashbin\BackgroundJob\ExpireTrash( + $backgroundJob = new ExpireTrash( $this->getMock('OCP\IDBConnection'), + $this->getMock('OCP\IUserManager'), $this->getMockBuilder('OCA\Files_Trashbin\Expiration')->disableOriginalConstructor()->getMock() ); From 764726ce0128054a036f8c515cc62d3ca2e490cd Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Fri, 11 Sep 2015 00:24:19 +0300 Subject: [PATCH 3/5] Updates according to review --- .../lib/backgroundjob/expiretrash.php | 32 +++++++++++++------ .../tests/backgroundjob/expiretrash.php | 2 +- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/apps/files_trashbin/lib/backgroundjob/expiretrash.php b/apps/files_trashbin/lib/backgroundjob/expiretrash.php index 350628444cf..07bcce0659b 100644 --- a/apps/files_trashbin/lib/backgroundjob/expiretrash.php +++ b/apps/files_trashbin/lib/backgroundjob/expiretrash.php @@ -21,7 +21,7 @@ namespace OCA\Files_Trashbin\BackgroundJob; -use OCP\IDBConnection; +use OCP\IConfig; use OCP\IUserManager; use OCA\Files_Trashbin\AppInfo\Application; use OCA\Files_Trashbin\Expiration; @@ -38,23 +38,25 @@ class ExpireTrash extends \OC\BackgroundJob\TimedJob { private $expiration; /** - * @var IDBConnection + * @var IConfig */ - private $dbConnection; + private $config; /** * @var IUserManager */ private $userManager; + + const USERS_PER_SESSION = 1000; - public function __construct(IDBConnection $dbConnection = null, IUserManager $userManager = null, Expiration $expiration = null) { + public function __construct(IConfig $config = null, IUserManager $userManager = null, Expiration $expiration = null) { // Run once per 30 minutes $this->setInterval(60 * 30); - if (is_null($expiration) || is_null($userManager) || is_null($dbConnection)) { + if (is_null($expiration) || is_null($userManager) || is_null($config)) { $this->fixDIForJobs(); } else { - $this->dbConnection = $dbConnection; + $this->config = $config; $this->userManager = $userManager; $this->expiration = $expiration; } @@ -62,7 +64,7 @@ class ExpireTrash extends \OC\BackgroundJob\TimedJob { protected function fixDIForJobs() { $application = new Application(); - $this->dbConnection = \OC::$server->getDatabaseConnection(); + $this->config = \OC::$server->getConfig(); $this->userManager = \OC::$server->getUserManager(); $this->expiration = $application->getContainer()->query('Expiration'); } @@ -72,17 +74,27 @@ class ExpireTrash extends \OC\BackgroundJob\TimedJob { if (!$maxAge) { return; } - $users = $this->userManager->search(''); + + $offset = $this->config->getAppValue('files_trashbin', 'cronjob_user_offset', 0); + $users = $this->userManager->search('', self::USERS_PER_SESSION, $offset); + if (!count($users)) { + // No users found, reset offset and retry + $offset = 0; + $users = $this->userManager->search('', self::USERS_PER_SESSION); + } + + $offset += self::USERS_PER_SESSION; + $this->config->setAppValue('files_trashbin', 'cronjob_user_offset', $offset); + foreach ($users as $user) { $uid = $user->getUID(); if (!$this->setupFS($uid)) { continue; } $dirContent = Helper::getTrashFiles('/', $uid, 'mtime'); - var_dump($dirContent); Trashbin::deleteExpiredFiles($dirContent, $uid); } - + \OC_Util::tearDownFS(); } diff --git a/apps/files_trashbin/tests/backgroundjob/expiretrash.php b/apps/files_trashbin/tests/backgroundjob/expiretrash.php index f39714f8a30..ad7b0fbca28 100644 --- a/apps/files_trashbin/tests/backgroundjob/expiretrash.php +++ b/apps/files_trashbin/tests/backgroundjob/expiretrash.php @@ -26,7 +26,7 @@ use \OCA\Files_Trashbin\BackgroundJob\ExpireTrash; class ExpireTrash_Test extends \Test\TestCase { public function testConstructAndRun() { $backgroundJob = new ExpireTrash( - $this->getMock('OCP\IDBConnection'), + $this->getMock('OCP\IConfig'), $this->getMock('OCP\IUserManager'), $this->getMockBuilder('OCA\Files_Trashbin\Expiration')->disableOriginalConstructor()->getMock() ); From 47caac10f5fd547e92b2668432f76313ee2a4f1a Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 15 Sep 2015 14:28:04 +0200 Subject: [PATCH 4/5] Add PHPDoc --- .../lib/backgroundjob/expiretrash.php | 13 ++++++++++++- apps/files_trashbin/lib/expiration.php | 5 ++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/apps/files_trashbin/lib/backgroundjob/expiretrash.php b/apps/files_trashbin/lib/backgroundjob/expiretrash.php index 07bcce0659b..a222767669a 100644 --- a/apps/files_trashbin/lib/backgroundjob/expiretrash.php +++ b/apps/files_trashbin/lib/backgroundjob/expiretrash.php @@ -49,7 +49,14 @@ class ExpireTrash extends \OC\BackgroundJob\TimedJob { const USERS_PER_SESSION = 1000; - public function __construct(IConfig $config = null, IUserManager $userManager = null, Expiration $expiration = null) { + /** + * @param IConfig|null $config + * @param IUserManager|null $userManager + * @param Expiration|null $expiration + */ + public function __construct(IConfig $config = null, + IUserManager $userManager = null, + Expiration $expiration = null) { // Run once per 30 minutes $this->setInterval(60 * 30); @@ -69,6 +76,10 @@ class ExpireTrash extends \OC\BackgroundJob\TimedJob { $this->expiration = $application->getContainer()->query('Expiration'); } + /** + * @param $argument + * @throws \Exception + */ protected function run($argument) { $maxAge = $this->expiration->getMaxAgeAsTimestamp(); if (!$maxAge) { diff --git a/apps/files_trashbin/lib/expiration.php b/apps/files_trashbin/lib/expiration.php index c8a6abb627b..5069521aab3 100644 --- a/apps/files_trashbin/lib/expiration.php +++ b/apps/files_trashbin/lib/expiration.php @@ -105,7 +105,10 @@ class Expiration { return $isOlderThanMax || $isMinReached; } - public function getMaxAgeAsTimestamp(){ + /** + * @return bool|int + */ + public function getMaxAgeAsTimestamp() { $maxAge = false; if ($this->isEnabled() && $this->maxAge !== self::NO_OBLIGATION) { $time = $this->timeFactory->getTime(); From b75a1b40a27ce5293930e673e531ebfa1a9cc81c Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Wed, 16 Sep 2015 22:06:57 +0300 Subject: [PATCH 5/5] Log deleted files --- apps/files_trashbin/lib/trashbin.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/files_trashbin/lib/trashbin.php b/apps/files_trashbin/lib/trashbin.php index 784a8e45b21..3b2d4cf5929 100644 --- a/apps/files_trashbin/lib/trashbin.php +++ b/apps/files_trashbin/lib/trashbin.php @@ -700,6 +700,10 @@ class Trashbin { if ($expiration->isExpired($timestamp)) { $count++; $size += self::delete($filename, $user, $timestamp); + \OC::$server->getLogger()->info( + 'Remove "' . $filename . '" from trashbin because it exceeds max retention obligation term.', + ['app' => 'files_trashbin'] + ); } else { break; }