Page MenuHomePhabricator
Files F13983758

D16243.id39072.diff
No OneTemporary
Actions

D16243.id39072.diff
View Options

diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php
--- a/src/applications/files/storage/PhabricatorFile.php
+++ b/src/applications/files/storage/PhabricatorFile.php
@@ -9,7 +9,7 @@
*
* | name | Human readable filename.
* | authorPHID | User PHID of uploader.
- * | ttl | Temporary file lifetime, in seconds.
+ * | ttl | Temporary file expiry time, seconds after epoch (UNIX timestamp).
* | viewPolicy | File visibility policy.
* | isExplicitUpload | Used to show users files they explicitly uploaded.
* | canCDN | Allows the file to be cached and delivered over a CDN.
@@ -203,6 +203,10 @@
* To solve these problems, we use file storage as a cache and reuse the
* same file again if we've previously written it.
*
+ * When a temporary file exists, its lifetime is extended to match the passed
+ * ttl parameter if necessary (including making the file permanent if no ttl
+ * is passed) so the file isn't unexpectedly deleted by the garbage collector.
+ *
* NOTE: This method unguards writes.
*
* @param string Raw file data.
@@ -221,6 +225,26 @@
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
$file = self::newFromFileData($data, $params);
unset($unguarded);
+
+ return $file;
+ }
+
+ $desired_ttl = idx($params, 'ttl');
+ $current_ttl = $file->getTtl();
+ if ($current_ttl !== null) {
+ if ($desired_ttl === null) {
+ $file->setTtl(null);
+
+ $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+ $file->save();
+ unset($unguarded);
+ } else if ($desired_ttl > $current_ttl) {
+ $file->setTtl($desired_ttl);
+
+ $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+ $file->save();
+ unset($unguarded);
+ }
}
return $file;
@@ -965,7 +989,7 @@
$builtins = mpull($builtins, null, 'getBuiltinFileKey');
$specs = array();
- foreach ($builtins as $key => $buitin) {
+ foreach ($builtins as $key => $builtin) {
$specs[] = array(
'originalPHID' => PhabricatorPHIDConstants::PHID_VOID,
'transform' => $key,
diff --git a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php
--- a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php
+++ b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php
@@ -384,6 +384,67 @@
$this->assertEqual($ttl, $file->getTTL());
}
+ public function testExtendFileTtl() {
+ $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);
+
+ // Newly-generated IDs are ints, but come back from the DB as strings
+ $id = (string)$file->getID();
+
+ // Ensure the file has its life extended if necessary
+
+ $ttl = (time() + 60 * 60 * 24 * 2);
+ $params['ttl'] = $ttl;
+
+ $file = PhabricatorFile::buildFromFileDataOrHash($data, $params);
+
+ $this->assertEqual($id, $file->getID());
+ $this->assertEqual($ttl, $file->getTTL());
+
+ // Ensure the file's life is not shortened
+
+ $params['ttl'] = (time() + 60 * 60 * 24);
+
+ $file = PhabricatorFile::buildFromFileDataOrHash($data, $params);
+
+ // Newly-set expiry times are ints, but come back from the DB as strings
+ $ttl = (string)$ttl;
+ $this->assertEqual($id, $file->getID());
+ $this->assertEqual($ttl, $file->getTTL());
+
+ // Ensure the file is made permanent if necessary
+
+ $ttl = null;
+ $params['ttl'] = $ttl;
+
+ $file = PhabricatorFile::buildFromFileDataOrHash($data, $params);
+
+ $this->assertEqual($id, $file->getID());
+ $this->assertEqual($ttl, $file->getTTL());
+
+ // Ensure the file is not made temporary
+
+ $params['ttl'] = (time() + 60 * 60 * 24);
+
+ $file = PhabricatorFile::buildFromFileDataOrHash($data, $params);
+
+ $this->assertEqual($id, $file->getID());
+ $this->assertEqual($ttl, $file->getTTL());
+ }
+
public function testFileTransformDelete() {
// We want to test that a file deletes all its inbound transformation
// records and outbound transformed derivatives when it is deleted.

File Metadata

Mime Type
text/plain
Expires
Mon, Oct 21, 8:30 AM (2 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6738885
Default Alt Text
D16243.id39072.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility