From b51a859636ac19bbcc160391b35e946ceaac4cba Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 21 Jan 2016 04:25:38 -0800 Subject: [PATCH] Allow diffusion.filecontentquery to load data for arbitrarily large files Summary: Fixes T10186. After D14970, `diffusion.filecontentquery` puts the content in a file and returns the file PHID. However, it does this in a way that doesn't go through the chunking engine, so it will fail for files larger than the chunk threshold (generally, 8MB). Instead, stream the file from the underlying command directly into chunked storage. Test Plan: - Made a commit including a really big file: https://github.com/epriestley/poems/commit/4dcd4c492b31d82f0fec6f447cf3719b9cc079e0 - Used `diffusion.filecontentquery` to load file content. - Parsed/imported commit locally. - Used `diffusion.filecontentquery` to load content for smaller files (README, etc). Reviewers: chad Reviewed By: chad Maniphest Tasks: T10186 Differential Revision: https://secure.phabricator.com/D15072 --- src/__phutil_library_map__.php | 4 + ...fusionFileContentQueryConduitAPIMethod.php | 32 +-- .../filecontent/DiffusionFileContentQuery.php | 33 ++- ...atorFilesApplicationStorageEnginePanel.php | 20 +- .../PhabricatorExecFutureFileUploadSource.php | 28 +++ .../PhabricatorFileUploadSource.php | 235 ++++++++++++++++++ 6 files changed, 309 insertions(+), 43 deletions(-) create mode 100644 src/applications/files/uploadsource/PhabricatorExecFutureFileUploadSource.php create mode 100644 src/applications/files/uploadsource/PhabricatorFileUploadSource.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d606f6c298..2b995de20d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2223,6 +2223,7 @@ phutil_register_library_map(array( 'PhabricatorEventListener' => 'infrastructure/events/PhabricatorEventListener.php', 'PhabricatorEventType' => 'infrastructure/events/constant/PhabricatorEventType.php', 'PhabricatorExampleEventListener' => 'infrastructure/events/PhabricatorExampleEventListener.php', + 'PhabricatorExecFutureFileUploadSource' => 'applications/files/uploadsource/PhabricatorExecFutureFileUploadSource.php', 'PhabricatorExtendedPolicyInterface' => 'applications/policy/interface/PhabricatorExtendedPolicyInterface.php', 'PhabricatorExtendingPhabricatorConfigOptions' => 'applications/config/option/PhabricatorExtendingPhabricatorConfigOptions.php', 'PhabricatorExtensionsSetupCheck' => 'applications/config/check/PhabricatorExtensionsSetupCheck.php', @@ -2312,6 +2313,7 @@ phutil_register_library_map(array( 'PhabricatorFileUploadController' => 'applications/files/controller/PhabricatorFileUploadController.php', 'PhabricatorFileUploadDialogController' => 'applications/files/controller/PhabricatorFileUploadDialogController.php', 'PhabricatorFileUploadException' => 'applications/files/exception/PhabricatorFileUploadException.php', + 'PhabricatorFileUploadSource' => 'applications/files/uploadsource/PhabricatorFileUploadSource.php', 'PhabricatorFileinfoSetupCheck' => 'applications/config/check/PhabricatorFileinfoSetupCheck.php', 'PhabricatorFilesApplication' => 'applications/files/application/PhabricatorFilesApplication.php', 'PhabricatorFilesApplicationStorageEnginePanel' => 'applications/files/applicationpanel/PhabricatorFilesApplicationStorageEnginePanel.php', @@ -6504,6 +6506,7 @@ phutil_register_library_map(array( 'PhabricatorEventListener' => 'PhutilEventListener', 'PhabricatorEventType' => 'PhutilEventType', 'PhabricatorExampleEventListener' => 'PhabricatorEventListener', + 'PhabricatorExecFutureFileUploadSource' => 'PhabricatorFileUploadSource', 'PhabricatorExtendingPhabricatorConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorExtensionsSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorExternalAccount' => array( @@ -6621,6 +6624,7 @@ phutil_register_library_map(array( 'PhabricatorFileUploadController' => 'PhabricatorFileController', 'PhabricatorFileUploadDialogController' => 'PhabricatorFileController', 'PhabricatorFileUploadException' => 'Exception', + 'PhabricatorFileUploadSource' => 'Phobject', 'PhabricatorFileinfoSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorFilesApplication' => 'PhabricatorApplication', 'PhabricatorFilesApplicationStorageEnginePanel' => 'PhabricatorApplicationConfigurationPanel', diff --git a/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php index 8088aa66b8..10e4d00b83 100644 --- a/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php @@ -39,14 +39,20 @@ final class DiffusionFileContentQueryConduitAPIMethod $file_query->setByteLimit($byte_limit); } - $content = $file_query->execute(); + $file = $file_query->execute(); $too_slow = (bool)$file_query->getExceededTimeLimit(); $too_huge = (bool)$file_query->getExceededByteLimit(); $file_phid = null; if (!$too_slow && !$too_huge) { - $file = $this->newFile($drequest, $content); + $repository = $drequest->getRepository(); + $repository_phid = $repository->getPHID(); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $file->attachToObject($repository_phid); + unset($unguarded); + $file_phid = $file->getPHID(); } @@ -57,26 +63,4 @@ final class DiffusionFileContentQueryConduitAPIMethod ); } - private function newFile(DiffusionRequest $drequest, $content) { - $path = $drequest->getPath(); - $name = basename($path); - - $repository = $drequest->getRepository(); - $repository_phid = $repository->getPHID(); - - $file = PhabricatorFile::buildFromFileDataOrHash( - $content, - array( - 'name' => $name, - 'ttl' => time() + phutil_units('48 hours in seconds'), - 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, - )); - - $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); - $file->attachToObject($repository_phid); - unset($unguarded); - - return $file; - } - } diff --git a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php index 3ea0e12ba1..df14b49235 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php @@ -54,23 +54,46 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery { $future->setStdoutSizeLimit($byte_limit + 1); } + $drequest = $this->getRequest(); + + $name = basename($drequest->getPath()); + $ttl = PhabricatorTime::getNow() + phutil_units('48 hours in seconds'); + try { - $file_content = $this->resolveFileContentFuture($future); + $threshold = PhabricatorFileStorageEngine::getChunkThreshold(); + $future->setReadBufferSize($threshold); + + $source = id(new PhabricatorExecFutureFileUploadSource()) + ->setName($name) + ->setTTL($ttl) + ->setViewPolicy(PhabricatorPolicies::POLICY_NOONE) + ->setExecFuture($future); + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $file = $source->uploadFile(); + unset($unguarded); + } catch (CommandException $ex) { if (!$future->getWasKilledByTimeout()) { throw $ex; } $this->didHitTimeLimit = true; - $file_content = null; + $file = null; } - if ($byte_limit && (strlen($file_content) > $byte_limit)) { + if ($byte_limit && ($file->getByteSize() > $byte_limit)) { $this->didHitByteLimit = true; - $file_content = null; + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + id(new PhabricatorDestructionEngine()) + ->destroyObject($file); + unset($unguarded); + + $file = null; } - return $file_content; + return $file; } } diff --git a/src/applications/files/applicationpanel/PhabricatorFilesApplicationStorageEnginePanel.php b/src/applications/files/applicationpanel/PhabricatorFilesApplicationStorageEnginePanel.php index c9a9bad51e..0d3fdc46cc 100644 --- a/src/applications/files/applicationpanel/PhabricatorFilesApplicationStorageEnginePanel.php +++ b/src/applications/files/applicationpanel/PhabricatorFilesApplicationStorageEnginePanel.php @@ -26,11 +26,15 @@ final class PhabricatorFilesApplicationStorageEnginePanel $rows = array(); $rowc = array(); foreach ($engines as $key => $engine) { - $limited = $no; + if ($engine->isTestEngine()) { + continue; + } + $limit = null; if ($engine->hasFilesizeLimit()) { - $limited = $yes; $limit = phutil_format_bytes($engine->getFilesizeLimit()); + } else { + $limit = pht('Unlimited'); } if ($engine->canWriteFiles()) { @@ -39,12 +43,6 @@ final class PhabricatorFilesApplicationStorageEnginePanel $writable = $no; } - if ($engine->isTestEngine()) { - $test = $yes; - } else { - $test = $no; - } - if (isset($writable_engines[$key]) || isset($chunk_engines[$key])) { $rowc[] = 'highlighted'; } else { @@ -54,9 +52,7 @@ final class PhabricatorFilesApplicationStorageEnginePanel $rows[] = array( $key, get_class($engine), - $test, $writable, - $limited, $limit, ); } @@ -67,9 +63,7 @@ final class PhabricatorFilesApplicationStorageEnginePanel array( pht('Key'), pht('Class'), - pht('Unit Test'), pht('Writable'), - pht('Has Limit'), pht('Limit'), )) ->setRowClasses($rowc) @@ -78,8 +72,6 @@ final class PhabricatorFilesApplicationStorageEnginePanel '', 'wide', '', - '', - '', 'n', )); diff --git a/src/applications/files/uploadsource/PhabricatorExecFutureFileUploadSource.php b/src/applications/files/uploadsource/PhabricatorExecFutureFileUploadSource.php new file mode 100644 index 0000000000..2e876c3e7a --- /dev/null +++ b/src/applications/files/uploadsource/PhabricatorExecFutureFileUploadSource.php @@ -0,0 +1,28 @@ +future = $future; + return $this; + } + + public function getExecFuture() { + return $this->future; + } + + protected function newDataIterator() { + $future = $this->getExecFuture(); + + return id(new LinesOfALargeExecFuture($future)) + ->setDelimiter(null); + } + + protected function getDataLength() { + return null; + } + +} diff --git a/src/applications/files/uploadsource/PhabricatorFileUploadSource.php b/src/applications/files/uploadsource/PhabricatorFileUploadSource.php new file mode 100644 index 0000000000..74b8e32389 --- /dev/null +++ b/src/applications/files/uploadsource/PhabricatorFileUploadSource.php @@ -0,0 +1,235 @@ +name = $name; + return $this; + } + + public function getName() { + return $this->name; + } + + public function setTTL($ttl) { + $this->ttl = $ttl; + return $this; + } + + public function getTTL() { + return $this->ttl; + } + + public function setViewPolicy($view_policy) { + $this->viewPolicy = $view_policy; + return $this; + } + + public function getViewPolicy() { + return $this->viewPolicy; + } + + public function uploadFile() { + if (!$this->shouldChunkFile()) { + return $this->writeSingleFile(); + } else { + return $this->writeChunkedFile(); + } + } + + private function getDataIterator() { + if (!$this->data) { + $this->data = $this->newDataIterator(); + } + return $this->data; + } + + private function getRope() { + if (!$this->rope) { + $this->rope = new PhutilRope(); + } + return $this->rope; + } + + abstract protected function newDataIterator(); + abstract protected function getDataLength(); + + private function readFileData() { + $data = $this->getDataIterator(); + + if (!$this->didRewind) { + $data->rewind(); + $this->didRewind = true; + } else { + $data->next(); + } + + if (!$data->valid()) { + return false; + } + + $rope = $this->getRope(); + $rope->append($data->current()); + + return true; + } + + private function shouldChunkFile() { + if ($this->shouldChunk !== null) { + return $this->shouldChunk; + } + + $threshold = PhabricatorFileStorageEngine::getChunkThreshold(); + + // If we don't know how large the file is, we're going to read some data + // from it until we know whether it's a small file or not. This will give + // us enough information to make a decision about chunking. + $length = $this->getDataLength(); + if ($length === null) { + $rope = $this->getRope(); + while ($this->readFileData()) { + $length = $rope->getByteLength(); + if ($length > $threshold) { + break; + } + } + } + + $this->shouldChunk = ($length > $threshold); + + return $this->shouldChunk; + } + + private function writeSingleFile() { + while ($this->readFileData()) { + // Read the entire file. + } + + $rope = $this->getRope(); + $data = $rope->getAsString(); + + $parameters = $this->getNewFileParameters(); + + return PhabricatorFile::newFromFileData($data, $parameters); + } + + private function writeChunkedFile() { + $engine = $this->getChunkEngine(); + + $parameters = $this->getNewFileParameters(); + + $parameters = array( + 'isPartial' => true, + ) + $parameters; + + $data_length = $this->getDataLength(); + if ($data_length !== null) { + $length = $data_length; + } else { + $length = 0; + } + + $file = PhabricatorFile::newChunkedFile($engine, $length, $parameters); + $file->save(); + + $rope = $this->getRope(); + + // Read the source, writing chunks as we get enough data. + while ($this->readFileData()) { + while (true) { + $rope_length = $rope->getByteLength(); + if ($rope_length < $engine->getChunkSize()) { + break; + } + $this->writeChunk($file, $engine); + } + } + + // If we have extra bytes at the end, write them. + if ($rope->getByteLength()) { + $this->writeChunk($file, $engine); + } + + $file->setIsPartial(0); + if ($data_length === null) { + $file->setByteSize($this->getTotalBytesWritten()); + } + $file->save(); + + return $file; + } + + private function writeChunk( + PhabricatorFile $file, + PhabricatorFileStorageEngine $engine) { + + $offset = $this->getTotalBytesWritten(); + $max_length = $engine->getChunkSize(); + $rope = $this->getRope(); + + $data = $rope->getPrefixBytes($max_length); + $actual_length = strlen($data); + $rope->removeBytesFromHead($actual_length); + + $chunk_data = PhabricatorFile::newFromFileData( + $data, + array( + 'name' => $file->getMonogram().'.chunk-'.$offset, + 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, + )); + + $chunk = PhabricatorFileChunk::initializeNewChunk( + $file->getStorageHandle(), + $offset, + $offset + $actual_length); + + $chunk + ->setDataFilePHID($chunk_data->getPHID()) + ->save(); + + $this->setTotalBytesWritten($offset + $actual_length); + + return $chunk; + } + + private function getNewFileParameters() { + return array( + 'name' => $this->getName(), + 'ttl' => $this->getTTL(), + 'viewPolicy' => $this->getViewPolicy(), + ); + } + + private function getChunkEngine() { + $chunk_engines = PhabricatorFileStorageEngine::loadWritableChunkEngines(); + if (!$chunk_engines) { + throw new Exception( + pht( + 'Unable to upload file: this server is not configured with any '. + 'storage engine which can store large files.')); + } + + return head($chunk_engines); + } + + private function setTotalBytesWritten($total_bytes_written) { + $this->totalBytesWritten = $total_bytes_written; + return $this; + } + + private function getTotalBytesWritten() { + return $this->totalBytesWritten; + } + +}