From c3e055549e79b59de19fac46d949f55225998f32 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Thu, 20 Aug 2015 18:32:41 +0300 Subject: [PATCH] Improvements --- apps/files_versions/lib/expiration.php | 37 +++++++++++++++++++----- apps/files_versions/lib/storage.php | 19 ++++++++---- apps/files_versions/tests/expiration.php | 8 +++-- config/config.sample.php | 2 +- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/apps/files_versions/lib/expiration.php b/apps/files_versions/lib/expiration.php index e1c7f50a20..02885e823c 100644 --- a/apps/files_versions/lib/expiration.php +++ b/apps/files_versions/lib/expiration.php @@ -27,7 +27,6 @@ use \OCP\AppFramework\Utility\ITimeFactory; class Expiration { // how long do we keep files a version if no other value is defined in the config file (unit: days) - const DEFAULT_RETENTION_OBLIGATION = 30; const NO_OBLIGATION = -1; /** @var ITimeFactory */ @@ -116,22 +115,44 @@ class Expiration { private function parseRetentionObligation(){ $splitValues = explode(',', $this->retentionObligation); if (!isset($splitValues[0])) { - $minValue = self::DEFAULT_RETENTION_OBLIGATION; + $minValue = 'auto'; } else { $minValue = trim($splitValues[0]); } - if (!isset($splitValues[1]) && $minValue === 'auto') { - $maxValue = 'auto'; - } elseif (!isset($splitValues[1])) { - $maxValue = self::DEFAULT_RETENTION_OBLIGATION; + if (!isset($splitValues[1])) { + $maxValue = self::NO_OBLIGATION; } else { $maxValue = trim($splitValues[1]); } + $isValid = true; + // Validate + if (!ctype_digit($minValue) && $minValue !== 'auto') { + $isValid = false; + \OC::$server->getLogger()->warning( + $minValue . ' is not a valid value for minimal versions retention obligation. Check versions_retention_obligation in your config.php. Falling back to auto.', + ['app'=>'files_versions'] + ); + } + + if (!ctype_digit($maxValue) && $maxValue !== 'auto') { + $isValid = false; + \OC::$server->getLogger()->warning( + $maxValue . ' is not a valid value for maximal versions retention obligation. Check versions_retention_obligation in your config.php. Falling back to auto.', + ['app'=>'files_versions'] + ); + } + + if (!$isValid){ + $minValue = 'auto'; + $maxValue = 'auto'; + } + + if ($minValue === 'auto' && $maxValue === 'auto') { - // Default: Keep for 30 days but delete anytime if space needed - $this->minAge = self::DEFAULT_RETENTION_OBLIGATION; + // Default: Delete anytime if space needed + $this->minAge = self::NO_OBLIGATION; $this->maxAge = self::NO_OBLIGATION; $this->canPurgeToSaveSpace = true; } elseif ($minValue !== 'auto' && $maxValue === 'auto') { diff --git a/apps/files_versions/lib/storage.php b/apps/files_versions/lib/storage.php index 269d43befb..90fe308e97 100644 --- a/apps/files_versions/lib/storage.php +++ b/apps/files_versions/lib/storage.php @@ -480,11 +480,11 @@ class Storage { * get list of files we want to expire * @param array $versions list of versions * @param integer $time + * @param bool $quotaExceeded is versions storage limit reached * @return array containing the list of to deleted versions and the size of them */ protected static function getExpireList($time, $versions, $quotaExceeded = false) { - $application = new Application(); - $expiration = $application->getContainer()->query('Expiration'); + $expiration = self::getExpiration(); if ($expiration->shouldAutoExpire()) { list($toDelete, $size) = self::getAutoExpireList($time, $versions); @@ -568,8 +568,7 @@ class Storage { */ private static function scheduleExpire($uid, $fileName, $versionsSize = null, $neededSpace = 0) { // let the admin disable auto expire - $application = new Application(); - $expiration = $application->getContainer()->query('Expiration'); + $expiration = self::getExpiration(); if ($expiration->isEnabled()) { $command = new Expire($uid, $fileName, $versionsSize, $neededSpace); \OC::$server->getCommandBus()->push($command); @@ -586,8 +585,7 @@ class Storage { */ public static function expire($filename, $versionsSize = null, $offset = 0) { $config = \OC::$server->getConfig(); - $application = new Application(); - $expiration = $application->getContainer()->query('Expiration'); + $expiration = self::getExpiration(); if($config->getSystemValue('files_versions', Storage::DEFAULTENABLED)=='true' && $expiration->isEnabled()) { list($uid, $filename) = self::getUidAndFilename($filename); @@ -706,4 +704,13 @@ class Storage { } } + /** + * Static workaround + * @return Expiration + */ + protected static function getExpiration(){ + $application = new Application(); + return $application->getContainer()->query('Expiration'); + } + } diff --git a/apps/files_versions/tests/expiration.php b/apps/files_versions/tests/expiration.php index 44d911c74b..fd5e022f68 100644 --- a/apps/files_versions/tests/expiration.php +++ b/apps/files_versions/tests/expiration.php @@ -111,13 +111,15 @@ class Expiration_Test extends \PHPUnit_Framework_TestCase { public function configData(){ return [ [ 'disabled', null, null, null], - [ 'auto', Expiration::DEFAULT_RETENTION_OBLIGATION, Expiration::NO_OBLIGATION, true ], - [ 'auto,auto', Expiration::DEFAULT_RETENTION_OBLIGATION, Expiration::NO_OBLIGATION, true ], - [ 'auto, auto', Expiration::DEFAULT_RETENTION_OBLIGATION, Expiration::NO_OBLIGATION, true ], + [ 'auto', Expiration::NO_OBLIGATION, Expiration::NO_OBLIGATION, true ], + [ 'auto,auto', Expiration::NO_OBLIGATION, Expiration::NO_OBLIGATION, true ], + [ 'auto, auto', Expiration::NO_OBLIGATION, Expiration::NO_OBLIGATION, true ], [ 'auto, 3', Expiration::NO_OBLIGATION, 3, true ], [ '5, auto', 5, Expiration::NO_OBLIGATION, true ], [ '3, 5', 3, 5, false ], [ '10, 3', 10, 10, false ], + [ 'g,a,r,b,a,g,e', Expiration::NO_OBLIGATION, Expiration::NO_OBLIGATION, true ], + [ '-3,8', Expiration::NO_OBLIGATION, Expiration::NO_OBLIGATION, true ] ]; } diff --git a/config/config.sample.php b/config/config.sample.php index a2889a57a7..a6d3fbcb65 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -451,7 +451,7 @@ $CONFIG = array( * ``auto`` default setting. keeps versions for 30 days and automatically * deletes anytime after that if space is needed (note: files * may not be deleted if space is not needed). - * ``D, auto`` keeps versions for D+ days, delete anytime if space needed + * ``D, auto`` keep versions for D+ days, delete anytime if space needed * (note: files may not be deleted * if space is not needed) * * ``auto, D`` delete all versions that are older than D days automatically,