From 762ace810df3730962cec9450e1022f611ee5399 Mon Sep 17 00:00:00 2001 From: kwadwo Date: Wed, 20 Feb 2013 13:33:47 -0800 Subject: [PATCH] Allow files to TTL and and be garbage collected Summary: Added ttl field to files. Gabage collect files with expired ttl Test Plan: created file with a ttl. Let garbage collector run Reviewers: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D4987 --- .../20130215.phabricatorfileaddttl.sql | 3 +++ .../DiffusionBrowseFileController.php | 1 + .../files/PhabricatorImageTransformer.php | 1 + .../PhabricatorFileInfoController.php | 8 ++++++++ .../PhabricatorFileListController.php | 7 +++++-- .../files/storage/PhabricatorFile.php | 20 ++++++++++++++++--- .../__tests__/PhabricatorFileTestCase.php | 19 ++++++++++++++++++ .../PhabricatorGarbageCollectorDaemon.php | 12 +++++++++++ .../patch/PhabricatorBuiltinPatchList.php | 4 ++++ 9 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 resources/sql/patches/20130215.phabricatorfileaddttl.sql diff --git a/resources/sql/patches/20130215.phabricatorfileaddttl.sql b/resources/sql/patches/20130215.phabricatorfileaddttl.sql new file mode 100644 index 0000000000..d8c3cbf8b3 --- /dev/null +++ b/resources/sql/patches/20130215.phabricatorfileaddttl.sql @@ -0,0 +1,3 @@ +ALTER TABLE {$NAMESPACE}_file.file + ADD ttl INT(10) UNSIGNED DEFAULT NULL, + ADD KEY key_ttl (ttl); diff --git a/src/applications/diffusion/controller/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/DiffusionBrowseFileController.php index 474605c44c..cb285fda77 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseFileController.php @@ -779,6 +779,7 @@ final class DiffusionBrowseFileController extends DiffusionController { $data, array( 'name' => basename($path), + 'ttl' => time() + 60 * 60 * 24, )); } diff --git a/src/applications/files/PhabricatorImageTransformer.php b/src/applications/files/PhabricatorImageTransformer.php index 80ff562bdb..c3a10e4816 100644 --- a/src/applications/files/PhabricatorImageTransformer.php +++ b/src/applications/files/PhabricatorImageTransformer.php @@ -11,6 +11,7 @@ final class PhabricatorImageTransformer { $image, array( 'name' => 'meme-'.$file->getName(), + 'ttl' => time() + 60 * 60 * 24, )); } diff --git a/src/applications/files/controller/PhabricatorFileInfoController.php b/src/applications/files/controller/PhabricatorFileInfoController.php index faeac9c154..fe031222b7 100644 --- a/src/applications/files/controller/PhabricatorFileInfoController.php +++ b/src/applications/files/controller/PhabricatorFileInfoController.php @@ -35,6 +35,14 @@ final class PhabricatorFileInfoController extends PhabricatorFileController { ->setObjectName('F'.$file->getID()) ->setHeader($file->getName()); + $ttl = $file->getTTL(); + if ($ttl !== null) { + $ttl_tag = id(new PhabricatorTagView()) + ->setType(PhabricatorTagView::TYPE_OBJECT) + ->setName(pht("Temporary")); + $header->addTag($ttl_tag); + } + $actions = $this->buildActionView($file); $properties = $this->buildPropertyView($file); diff --git a/src/applications/files/controller/PhabricatorFileListController.php b/src/applications/files/controller/PhabricatorFileListController.php index 5e31eb3554..a2dd69e08d 100644 --- a/src/applications/files/controller/PhabricatorFileListController.php +++ b/src/applications/files/controller/PhabricatorFileListController.php @@ -89,12 +89,10 @@ final class PhabricatorFileListController extends PhabricatorFileController { $id = $file->getID(); $phid = $file->getPHID(); $name = $file->getName(); - $file_name = "F{$id} {$name}"; $file_uri = $this->getApplicationURI("/info/{$phid}/"); $date_created = phabricator_date($file->getDateCreated(), $user); - $author_phid = $file->getAuthorPHID(); if ($author_phid) { $author_link = $this->getHandle($author_phid)->renderLink(); @@ -110,6 +108,11 @@ final class PhabricatorFileListController extends PhabricatorFileController { ->addAttribute($uploaded) ->addIcon('none', phabricator_format_bytes($file->getByteSize())); + $ttl = $file->getTTL(); + if ($ttl !== null) { + $item->addIcon('blame', pht('Temporary')); + } + if (isset($highlighted_ids[$id])) { $item->setEffect('highlighted'); } diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index d74f995b42..fee670dd66 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -21,6 +21,8 @@ final class PhabricatorFile extends PhabricatorFileDAO protected $storageFormat; protected $storageHandle; + protected $ttl; + public function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, @@ -148,6 +150,7 @@ final class PhabricatorFile extends PhabricatorFileDAO $file_name = idx($params, 'name'); $file_name = self::normalizeFileName($file_name); + $file_ttl = idx($params, 'ttl'); $authorPHID = idx($params, 'authorPHID'); $new_file = new PhabricatorFile(); @@ -155,6 +158,7 @@ final class PhabricatorFile extends PhabricatorFileDAO $new_file->setName($file_name); $new_file->setByteSize($copy_of_byteSize); $new_file->setAuthorPHID($authorPHID); + $new_file->setTtl($file_ttl); $new_file->setContentHash($hash); $new_file->setStorageEngine($copy_of_storage_engine); @@ -223,6 +227,7 @@ final class PhabricatorFile extends PhabricatorFileDAO $file_name = idx($params, 'name'); $file_name = self::normalizeFileName($file_name); + $file_ttl = idx($params, 'ttl'); // If for whatever reason, authorPHID isn't passed as a param // (always the case with newFromFileDownload()), store a '' @@ -231,6 +236,7 @@ final class PhabricatorFile extends PhabricatorFileDAO $file->setName($file_name); $file->setByteSize(strlen($data)); $file->setAuthorPHID($authorPHID); + $file->setTtl($file_ttl); $file->setContentHash(self::hashFileContent($data)); $file->setStorageEngine($engine_identifier); @@ -354,6 +360,17 @@ final class PhabricatorFile extends PhabricatorFileDAO } public function delete() { + // delete all records of this file in transformedfile + $trans_files = id(new PhabricatorTransformedFile())->loadAllWhere( + 'TransformedPHID = %s', $this->getPHID()); + + $this->openTransaction(); + foreach ($trans_files as $trans_file) { + $trans_file->delete(); + } + $ret = parent::delete(); + $this->saveTransaction(); + // Check to see if other files are using storage $other_file = id(new PhabricatorFile())->loadAllWhere( 'storageEngine = %s AND storageHandle = %s AND @@ -366,9 +383,6 @@ final class PhabricatorFile extends PhabricatorFileDAO $engine = $this->instantiateStorageEngine(); $engine->deleteFile($this->getStorageHandle()); } - - $ret = parent::delete(); - return $ret; } diff --git a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php index f794585080..d2b2112c0f 100644 --- a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php +++ b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php @@ -126,4 +126,23 @@ final class PhabricatorFileTestCase extends PhabricatorTestCase { $this->assertEqual($data, $second_file->loadFileData()); } + public function testReadWriteTtlFiles() { + $engine = new PhabricatorTestStorageEngine(); + + $data = Filesystem::readRandomCharacters(64); + + $ttl = (time() + 60 * 60 * 24); + + $params = array( + 'name' => 'test.dat', + 'ttl' => ($ttl), + 'storageEngines' => array( + $engine, + ), + ); + + $file = PhabricatorFile::newFromFileData($data, $params); + $this->assertEqual($ttl, $file->getTTL()); + } + } diff --git a/src/infrastructure/daemon/PhabricatorGarbageCollectorDaemon.php b/src/infrastructure/daemon/PhabricatorGarbageCollectorDaemon.php index f4f678b14c..dc736e8982 100644 --- a/src/infrastructure/daemon/PhabricatorGarbageCollectorDaemon.php +++ b/src/infrastructure/daemon/PhabricatorGarbageCollectorDaemon.php @@ -17,6 +17,7 @@ final class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon { $n_tasks = $this->collectArchivedTasks(); $n_cache_ttl = $this->collectGeneralCacheTTL(); $n_cache = $this->collectGeneralCaches(); + $n_files = $this->collectExpiredFiles(); $collected = array( 'Herald Transcript' => $n_herald, @@ -26,6 +27,7 @@ final class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon { 'Archived Tasks' => $n_tasks, 'General Cache TTL' => $n_cache_ttl, 'General Cache Entries' => $n_cache, + 'Temporary Files' => $n_files, ); $collected = array_filter($collected); @@ -206,5 +208,15 @@ final class PhabricatorGarbageCollectorDaemon extends PhabricatorDaemon { return $conn_w->getAffectedRows(); } + private function collectExpiredFiles() { + $files = id(new PhabricatorFile())->loadAllWhere('ttl < %d LIMIT 100', + time()); + + foreach ($files as $file) { + $file->delete(); + } + + return count($files); + } } diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index dd677403b1..f678f7c4cb 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1137,6 +1137,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130218.longdaemon.sql'), ), + '20130215.phabricatorfileaddttl.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20130215.phabricatorfileaddttl.sql'), + ), ); }