1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-14 02:42:40 +01:00

Add support for partially uploaded files

Summary:
Ref T7149. This flags allocated but incomplete files and doesn't explode when trying to download them.

Files are marked complete when the last chunk is uploaded.

I added a key on `<authorPHID, isPartial>` so we can show you a list of partially uploaded files and prompt you to resume them at some point down the road.

Test Plan: Massaged debugging settings and uploaded README.md very slowly in 32b chunks. Saw the file lose its "Partial" flag when the last chunk finished.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: joshuaspence, epriestley

Maniphest Tasks: T7149

Differential Revision: https://secure.phabricator.com/D12063
This commit is contained in:
epriestley 2015-03-13 11:30:24 -07:00
parent 6c3552f939
commit aa4adf3ab8
9 changed files with 136 additions and 58 deletions

View file

@ -0,0 +1,2 @@
ALTER TABLE {$NAMESPACE}_file.file
ADD isPartial BOOL NOT NULL DEFAULT 0;

View file

@ -0,0 +1,2 @@
ALTER TABLE {$NAMESPACE}_file.file
ADD KEY `key_partial` (authorPHID, isPartial);

View file

@ -53,7 +53,7 @@ final class FileAllocateConduitAPIMethod
$hash, $hash,
$properties); $properties);
if ($file && !$force_chunking) { if ($file) {
return array( return array(
'upload' => false, 'upload' => false,
'filePHID' => $file->getPHID(), 'filePHID' => $file->getPHID(),
@ -70,7 +70,7 @@ final class FileAllocateConduitAPIMethod
if ($file) { if ($file) {
return array( return array(
'upload' => $file->isPartial(), 'upload' => (bool)$file->getIsPartial(),
'filePHID' => $file->getPHID(), 'filePHID' => $file->getPHID(),
); );
} }
@ -103,7 +103,7 @@ final class FileAllocateConduitAPIMethod
// Otherwise, this is a large file and we need to perform a chunked // Otherwise, this is a large file and we need to perform a chunked
// upload. // upload.
$chunk_properties = array(); $chunk_properties = $properties;
if ($hash) { if ($hash) {
$chunk_properties += array( $chunk_properties += array(

View file

@ -89,6 +89,16 @@ abstract class FileConduitAPIMethod extends ConduitAPIMethod {
return $data; return $data;
} }
protected function loadAnyMissingChunk(
PhabricatorUser $viewer,
PhabricatorFile $file) {
return $this->newChunkQuery($viewer, $file)
->withIsComplete(false)
->setLimit(1)
->execute();
}
private function newChunkQuery( private function newChunkQuery(
PhabricatorUser $viewer, PhabricatorUser $viewer,
PhabricatorFile $file) { PhabricatorFile $file) {
@ -106,4 +116,5 @@ abstract class FileConduitAPIMethod extends ConduitAPIMethod {
->withChunkHandles(array($file->getStorageHandle())); ->withChunkHandles(array($file->getStorageHandle()));
} }
} }

View file

@ -57,16 +57,19 @@ final class FileUploadChunkConduitAPIMethod
// NOTE: These files have a view policy which prevents normal access. They // NOTE: These files have a view policy which prevents normal access. They
// are only accessed through the storage engine. // are only accessed through the storage engine.
$file = PhabricatorFile::newFromFileData( $chunk_data = PhabricatorFile::newFromFileData(
$data, $data,
array( array(
'name' => $file->getMonogram().'.chunk-'.$chunk->getID(), 'name' => $file->getMonogram().'.chunk-'.$chunk->getID(),
'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE,
)); ));
$chunk->setDataFilePHID($file->getPHID())->save(); $chunk->setDataFilePHID($chunk_data->getPHID())->save();
// TODO: If all chunks are up, mark the file as complete. $missing = $this->loadAnyMissingChunk($viewer, $file);
if (!$missing) {
$file->setIsPartial(0)->save();
}
return null; return null;
} }

View file

@ -5,6 +5,7 @@ final class PhabricatorFileDataController extends PhabricatorFileController {
private $phid; private $phid;
private $key; private $key;
private $token; private $token;
private $file;
public function willProcessRequest(array $data) { public function willProcessRequest(array $data) {
$this->phid = $data['phid']; $this->phid = $data['phid'];
@ -16,20 +17,9 @@ final class PhabricatorFileDataController extends PhabricatorFileController {
return false; return false;
} }
protected function checkFileAndToken($file) {
if (!$file) {
return new Aphront404Response();
}
if (!$file->validateSecretKey($this->key)) {
return new Aphront403Response();
}
return null;
}
public function processRequest() { public function processRequest() {
$request = $this->getRequest(); $request = $this->getRequest();
$viewer = $this->getViewer();
$alt = PhabricatorEnv::getEnvConfig('security.alternate-file-domain'); $alt = PhabricatorEnv::getEnvConfig('security.alternate-file-domain');
$base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri'); $base_uri = PhabricatorEnv::getEnvConfig('phabricator.base-uri');
@ -44,32 +34,22 @@ final class PhabricatorFileDataController extends PhabricatorFileController {
// Alternate files domain isn't configured or it's set // Alternate files domain isn't configured or it's set
// to the same as the default domain // to the same as the default domain
// load the file with permissions checks; $response = $this->loadFile($viewer);
$file = id(new PhabricatorFileQuery()) if ($response) {
->setViewer($request->getUser()) return $response;
->withPHIDs(array($this->phid))
->executeOne();
$error_response = $this->checkFileAndToken($file);
if ($error_response) {
return $error_response;
} }
$file = $this->getFile();
// when the file is not CDNable, don't allow cache // when the file is not CDNable, don't allow cache
$cache_response = $file->getCanCDN(); $cache_response = $file->getCanCDN();
} else if ($req_domain != $alt_domain) { } else if ($req_domain != $alt_domain) {
// Alternate domain is configured but this request isn't using it // Alternate domain is configured but this request isn't using it
// load the file with permissions checks; $response = $this->loadFile($viewer);
$file = id(new PhabricatorFileQuery()) if ($response) {
->setViewer($request->getUser()) return $response;
->withPHIDs(array($this->phid))
->executeOne();
$error_response = $this->checkFileAndToken($file);
if ($error_response) {
return $error_response;
} }
$file = $this->getFile();
// if the user can see the file, generate a token; // if the user can see the file, generate a token;
// redirect to the alt domain with the token; // redirect to the alt domain with the token;
@ -82,18 +62,15 @@ final class PhabricatorFileDataController extends PhabricatorFileController {
->setURI($token_uri); ->setURI($token_uri);
} else { } else {
// We are using the alternate domain // We are using the alternate domain. We don't have authentication
// on this domain, so we bypass policy checks when loading the file.
// load the file, bypassing permission checks; $bypass_policies = PhabricatorUser::getOmnipotentUser();
$file = id(new PhabricatorFileQuery()) $response = $this->loadFile($bypass_policies);
->setViewer(PhabricatorUser::getOmnipotentUser()) if ($response) {
->withPHIDs(array($this->phid)) return $response;
->executeOne();
$error_response = $this->checkFileAndToken($file);
if ($error_response) {
return $error_response;
} }
$file = $this->getFile();
$acquire_token_uri = id(new PhutilURI($file->getViewURI())) $acquire_token_uri = id(new PhutilURI($file->getViewURI()))
->setDomain($main_domain); ->setDomain($main_domain);
@ -205,4 +182,44 @@ final class PhabricatorFileDataController extends PhabricatorFileController {
return $uri; return $uri;
} }
private function loadFile(PhabricatorUser $viewer) {
$file = id(new PhabricatorFileQuery())
->setViewer($viewer)
->withPHIDs(array($this->phid))
->executeOne();
if (!$file) {
return new Aphront404Response();
}
if (!$file->validateSecretKey($this->key)) {
return new Aphront403Response();
}
if ($file->getIsPartial()) {
// We may be on the CDN domain, so we need to use a fully-qualified URI
// here to make sure we end up back on the main domain.
$info_uri = PhabricatorEnv::getURI($file->getInfoURI());
return $this->newDialog()
->setTitle(pht('Partial Upload'))
->appendParagraph(
pht(
'This file has only been partially uploaded. It must be '.
'uploaded completely before you can download it.'))
->addCancelButton($info_uri);
}
$this->file = $file;
return null;
}
private function getFile() {
if (!$this->file) {
throw new Exception(pht('Call loadFile() before getFile()!'));
}
return $this->file;
}
} }

View file

@ -52,11 +52,21 @@ final class PhabricatorFileInfoController extends PhabricatorFileController {
$ttl = $file->getTTL(); $ttl = $file->getTTL();
if ($ttl !== null) { if ($ttl !== null) {
$ttl_tag = id(new PHUITagView()) $ttl_tag = id(new PHUITagView())
->setType(PHUITagView::TYPE_OBJECT) ->setType(PHUITagView::TYPE_STATE)
->setBackgroundColor(PHUITagView::COLOR_YELLOW)
->setName(pht('Temporary')); ->setName(pht('Temporary'));
$header->addTag($ttl_tag); $header->addTag($ttl_tag);
} }
$partial = $file->getIsPartial();
if ($partial) {
$partial_tag = id(new PHUITagView())
->setType(PHUITagView::TYPE_STATE)
->setBackgroundColor(PHUITagView::COLOR_ORANGE)
->setName(pht('Partial Upload'));
$header->addTag($partial_tag);
}
$actions = $this->buildActionView($file); $actions = $this->buildActionView($file);
$timeline = $this->buildTransactionView($file); $timeline = $this->buildTransactionView($file);
$crumbs = $this->buildApplicationCrumbs(); $crumbs = $this->buildApplicationCrumbs();
@ -126,21 +136,27 @@ final class PhabricatorFileInfoController extends PhabricatorFileController {
->setObjectURI($this->getRequest()->getRequestURI()) ->setObjectURI($this->getRequest()->getRequestURI())
->setObject($file); ->setObject($file);
$can_download = !$file->getIsPartial();
if ($file->isViewableInBrowser()) { if ($file->isViewableInBrowser()) {
$view->addAction( $view->addAction(
id(new PhabricatorActionView()) id(new PhabricatorActionView())
->setName(pht('View File')) ->setName(pht('View File'))
->setIcon('fa-file-o') ->setIcon('fa-file-o')
->setHref($file->getViewURI())); ->setHref($file->getViewURI())
->setDisabled(!$can_download)
->setWorkflow(!$can_download));
} else { } else {
$view->addAction( $view->addAction(
id(new PhabricatorActionView()) id(new PhabricatorActionView())
->setUser($viewer) ->setUser($viewer)
->setRenderAsForm(true) ->setRenderAsForm($can_download)
->setDownload(true) ->setDownload($can_download)
->setName(pht('Download File')) ->setName(pht('Download File'))
->setIcon('fa-download') ->setIcon('fa-download')
->setHref($file->getViewURI())); ->setHref($file->getViewURI())
->setDisabled(!$can_download)
->setWorkflow(!$can_download));
} }
$view->addAction( $view->addAction(

View file

@ -6,6 +6,7 @@ final class PhabricatorFileChunkQuery
private $chunkHandles; private $chunkHandles;
private $rangeStart; private $rangeStart;
private $rangeEnd; private $rangeEnd;
private $isComplete;
private $needDataFiles; private $needDataFiles;
public function withChunkHandles(array $handles) { public function withChunkHandles(array $handles) {
@ -19,6 +20,11 @@ final class PhabricatorFileChunkQuery
return $this; return $this;
} }
public function withIsComplete($complete) {
$this->isComplete = $complete;
return $this;
}
public function needDataFiles($need) { public function needDataFiles($need) {
$this->needDataFiles = $need; $this->needDataFiles = $need;
return $this; return $this;
@ -104,6 +110,18 @@ final class PhabricatorFileChunkQuery
$this->rangeEnd); $this->rangeEnd);
} }
if ($this->isComplete !== null) {
if ($this->isComplete) {
$where[] = qsprintf(
$conn_r,
'dataFilePHID IS NOT NULL');
} else {
$where[] = qsprintf(
$conn_r,
'dataFilePHID IS NULL');
}
}
$where[] = $this->buildPagingClause($conn_r); $where[] = $this->buildPagingClause($conn_r);
return $this->formatWhereClause($where); return $this->formatWhereClause($where);

View file

@ -51,6 +51,7 @@ final class PhabricatorFile extends PhabricatorFileDAO
protected $ttl; protected $ttl;
protected $isExplicitUpload = 1; protected $isExplicitUpload = 1;
protected $viewPolicy = PhabricatorPolicies::POLICY_USER; protected $viewPolicy = PhabricatorPolicies::POLICY_USER;
protected $isPartial = 0;
private $objects = self::ATTACHABLE; private $objects = self::ATTACHABLE;
private $objectPHIDs = self::ATTACHABLE; private $objectPHIDs = self::ATTACHABLE;
@ -67,6 +68,7 @@ final class PhabricatorFile extends PhabricatorFileDAO
return id(new PhabricatorFile()) return id(new PhabricatorFile())
->setViewPolicy($view_policy) ->setViewPolicy($view_policy)
->setIsPartial(0)
->attachOriginalFile(null) ->attachOriginalFile(null)
->attachObjects(array()) ->attachObjects(array())
->attachObjectPHIDs(array()); ->attachObjectPHIDs(array());
@ -91,6 +93,7 @@ final class PhabricatorFile extends PhabricatorFileDAO
'ttl' => 'epoch?', 'ttl' => 'epoch?',
'isExplicitUpload' => 'bool?', 'isExplicitUpload' => 'bool?',
'mailKey' => 'bytes20', 'mailKey' => 'bytes20',
'isPartial' => 'bool',
), ),
self::CONFIG_KEY_SCHEMA => array( self::CONFIG_KEY_SCHEMA => array(
'key_phid' => null, 'key_phid' => null,
@ -110,6 +113,9 @@ final class PhabricatorFile extends PhabricatorFileDAO
'key_dateCreated' => array( 'key_dateCreated' => array(
'columns' => array('dateCreated'), 'columns' => array('dateCreated'),
), ),
'key_partial' => array(
'columns' => array('authorPHID', 'isPartial'),
),
), ),
) + parent::getConfiguration(); ) + parent::getConfiguration();
} }
@ -294,6 +300,7 @@ final class PhabricatorFile extends PhabricatorFileDAO
$file->setStorageEngine($engine->getEngineIdentifier()); $file->setStorageEngine($engine->getEngineIdentifier());
$file->setStorageHandle(PhabricatorFileChunk::newChunkHandle()); $file->setStorageHandle(PhabricatorFileChunk::newChunkHandle());
$file->setStorageFormat(self::STORAGE_FORMAT_RAW); $file->setStorageFormat(self::STORAGE_FORMAT_RAW);
$file->setIsPartial(1);
$file->readPropertiesFromParameters($params); $file->readPropertiesFromParameters($params);
@ -628,10 +635,17 @@ final class PhabricatorFile extends PhabricatorFileDAO
} }
$parts[] = $name; $parts[] = $name;
$path = implode('/', $parts); $path = '/'.implode('/', $parts);
// If this file is only partially uploaded, we're just going to return a
// local URI to make sure that Ajax works, since the page is inevitably
// going to give us an error back.
if ($this->getIsPartial()) {
return PhabricatorEnv::getURI($path);
} else {
return PhabricatorEnv::getCDNURI($path); return PhabricatorEnv::getCDNURI($path);
} }
}
/** /**
* Get the CDN URI for this file, including a one-time-use security token. * Get the CDN URI for this file, including a one-time-use security token.
@ -1170,11 +1184,6 @@ final class PhabricatorFile extends PhabricatorFileDAO
->setURI($uri); ->setURI($uri);
} }
public function isPartial() {
// TODO: Placeholder for resumable uploads.
return false;
}
/* -( PhabricatorApplicationTransactionInterface )------------------------- */ /* -( PhabricatorApplicationTransactionInterface )------------------------- */